I gave a try to the last sqlsvr dev version (08/24) and I got the following PDO Exception :

PDOException : SQLSTATE[42000]: [Microsoft][SQL Server Native Client 10.0][SQL Server]Impossible de trouver la colonne "dbo" la fonction définie par l'utilisateur ou l'agrégat "dbo.SUBSTRING". Le nom pourrait également être ambigu.: SELECT TOP(50) c.[cid] AS [cid], SUBSTRING(c.thread, 1, (LENGTH(c.thread) - 1)) AS torder FROM {comment} c WHERE ( ([c].[nid] = :db_condition_placeholder_0) ) ORDER BY torder ASC; Array ( [:db_condition_placeholder_0] => 5 ) dans PagerDefault->execute() (ligne 79 dans C:\PrintKlub\SiteWeb\includes\pager.inc).

The page has only a very basic view and was running fine with sqlsrv 7.x-1.1. I hope this could ring the bell...

Comments

Artusamak’s picture

Status: Active » Closed (duplicate)
Uncle_Code_Monkey’s picture

Status: Closed (duplicate) » Active

This is not a duplicate of SUBSTRING() having incorrect arguments. This is a variation of an error where the SUBSTRING function itself is missing from the database altogether, as Damien Tournoud pointed out in his comment. I just ran into this bug not because my database became corrupted, but because I updated an older sqlsrv driver that did not have SUBSTRING() defined, but now it does in the latest driver code.

The problem is that the recommended solution to "just visit update.php" so that the missing function(s) will get recreated does not work with the latest driver. I am not the only one that has run into this problem either.

I am in the process of fixing quite a lot of issues with the sqlsrv driver, so if someone would be so kind as to point me in the right direction on what I should look at to fix this update.php issue, I would appreciate it.

Damien Tournoud’s picture

@Uncle Code Monkey: visit install.php, not update.php.

xenphibian’s picture

A fix hack for this problem is to add 'dbo.' in front of user defined functions.

The problem is that the sqlsrv driver isn't properly prepending 'dbo.' to function names.

In the referenced problem the database correctly automatically prepends 'dbo.' to SUBSTRING (SUBSTRING => dbo.SUBSTRING in the database), but does NOT do so for LENGTH.

As a result you get a query that looks like:

SELECT TOP(50) 
	c.[cid] AS [cid], 
	SUBSTRING(c.thread, 1, (LENGTH(c.thread) - 1)) AS torder 
FROM comment c 
WHERE ( ([c].[nid] = 360) ) 
ORDER BY torder ASC

Which DOES NOT operate correctly (I tested it to find out what the real, underlying problem was). This produces the error

Msg 195, Level 15, State 10, Line 3
'LENGTH' is not a recognized built-in function name.

You SHOULD get a query that looks like:

SELECT TOP(50) 
	c.[cid] AS [cid], 
	dbo.SUBSTRING(c.thread, 1, (dbo.LENGTH(c.thread) - 1)) AS torder 
FROM comment c 
WHERE ( ([c].[nid] = 360) ) 
ORDER BY torder ASC

There is no mechanism I can find that escapes function calls. This could be easily added (assuming that it actually doesn't already exist and I'm just not finding it) by replacing "LENGTH(" with "dbo.LENGTH(" and so on for all of the db driver defined functions. And, possibly for other UDFs if it is a carefully designed replacement (an array of strings that modules can add to, rather than a fixed replacement).

Uncle_Code_Monkey’s picture

Status: Active » Closed (works as designed)

@Damien Thanks, visiting install.php did indeed fix the problem by redefining all the necessary functions, including adding the missing SUBSTRING one. Thank you. I am going to mark this Closed(Works as designed) since no fix is necessary.

@xenphibian The sqlsrv driver (7.x-1.2 release & dev versions) replaces all LENGTH() calls it finds in queries with LEN() calls. It should not be converting it into dbo.LENGTH() because no such function has been defined by the driver, nor exists.

See the sqlsrv/database.inc file, preprocessQuery() function. The following lines do the conversion:

    $replacements = array(
      'LENGTH' => 'LEN',
      'POW' => 'POWER',
    );
    foreach ($replacements as $function => $replacement) {
      $query = preg_replace('/\b(?<![:.])(' . preg_quote($function) . ')\(/i', $replacement . '(', $query);
    }
xenphibian’s picture

The function DOES exist. I have removed it manually and will now test with that. I'm guessing that I had an older driver version (probably from a complete package that I was playing with from a while back) when my sandbox database was created.

But, the functionality that I was looking for does also exist, in a sort of kind of way. The list of functions should be extendable and not just limited to Drupal core ones.

Uncle_Code_Monkey’s picture

If you look just above that code I mentioned in #5, you should see this section:

    // Add prefixes to Drupal-specific functions.
    $functions = array(
      'SUBSTRING',
      'SUBSTRING_INDEX',
      'GREATEST',
      'CONCAT',
      'IF',
    );
    foreach ($functions as $function) {
      $query = preg_replace('/\b(?<![:.])(' . preg_quote($function) . ')\(/i', $this->schema()->defaultSchema . '.$1(', $query);
    }

You could add your function to that array for the time being or help extend the module by getting someone to write the Configuration UI code (which, since it's also a module in the standard module list could easily be done). Something like a user configurable array that gets merged with the pre-defined one, maybe. All of the pre-defined SQL functions exist in the install.inc file and custom ones you want to put in the driver would go there ... but that would be optional since custom functions can just be defined/created in the database itself, too.

xenphibian’s picture

Yep, I already saw that and was already considering options for how to implement exactly what you're mentioning.

The approach I'm testing before offering it up for review looks something like this:

    // Add prefixes to Drupal-specific functions.
	/*
    $functions = array(
      'SUBSTRING',
      'SUBSTRING_INDEX',
      'GREATEST',
      'CONCAT',
      'IF',
    );
	*/
	
	if (!isset($moduleSQLFunctions)) {
	  $moduleSQLFunctions = array();
	}

	$globalDrupalFunctions = array(
      'SUBSTRING',
      'SUBSTRING_INDEX',
      'GREATEST',
      'CONCAT',
      'IF',
    );

	$functions = array_unique(array_merge($globalDrupalFunctions, $moduleSQLFunctions));

The rest of the code would be identical.

Then, a module wanting to use functions could define an array $moduleSQLFunctions that holds the list of functions to add for any given query. This should allow quite fine control over the functions that are defined/used including the ability to detect/remove/redefine/etc. any function names that collide with other strings in the query for whatever reason.

This is not optimal yet, but I want to test it and play with it before I optimize and then submit it. And, I realize that I'm not the usual Drupal user in that I've got quite a bit of professional programming experience (20-ish years of it, mostly C++ and SQL).

xenphibian’s picture

Oh, and as for install code, I was planning to put that in the module.install code under the hook_install() implementation.

I really do think that we could see some VAST improvements in performance for modules that are really hurting for performance by doing this simple sort of thing. In the past I have made queries that run for more than 5 minutes drop down to below 30 seconds with the same data set and result set.

Damien Tournoud’s picture

So no, there is no LENGTH user-defined function. We just replace LENGTH() with LEN() before running the query. As far as I remember, there never been a LENGTH function in the driver.

Drupal requires the database to be leveled, so we implement a handful of compatibility functions. This is a closed list. It is a bug for a module to use any other non-portable function. I wish we could avoid this prefixing, but SQL Server has some braindead rules for unprefixed objects (namely: it will look for an unprefixed object in the current default schema for mostly everything *except* user-defined functions).

xenphibian’s picture

RE: LENGTH function -- I didn't write it but it was in the driver that I had been using for a while now (can't remember where that driver came from, but I'm obviously not the only one who ran into this...).

RE: Leveled DB -- Yes, I know. As much as I love SQL Server, I am saddened that the name resolution mechanism is so limited that it is unable to determine that there is no system defined scalar function for a function call, but there IS a user defined one. This has been the case for many years and there has been a very big complaint about it in the SQL Server community for a long time but Microsoft never responds to the complaint, meaning that this is intentional and not an oversight.

The mechanism that I have described above will allow for PORTABLE function calls. I fully agree with the requirement that any function a module uses should be portable (meaning that it is either defined for all supported databases, or that it is not used). But the requirement that NO functions are used is a bad, bad idea. And, the lack of use of stored procedures is even a worse idea. I have personally battled this out on the C++ front for years saying that C++ can be so much more efficient than any database can be, but I finally admitted defeat when generated queries were soundly beaten by a factor of as much as 20:1 by stored procedures. And, that's on all tested platforms (SQL Server, SQL Express, MySQL, PostgreSQL, Oracle, Sybase, and a couple of others that I didn't know at all).

That said, if we are calling it a bug for a module to not be compatible with any database module out there then the response that I have seen, "We're not currently compatible with SQL Server, no plans to fix," is a bug for which the module builders should be spanked. I had to port the privatemsg module myself which got exactly this response. It only involved a few simple changes to the module as well as a simple change to sqlsrv/schema.inc (escaping the field names for an unsigned check constraint, which should have been done anyway).

Damien Tournoud’s picture

Well, that's just the way it is. We are not going to introduce even more special cases in the driver to workaround bugs in modules.

... as well as a simple change to sqlsrv/schema.inc (escaping the field names for an unsigned check constraint, which should have been done anyway).

That sounds familiar. That was either recently fixed or we have an issue for it, right?

xenphibian’s picture

Well, that's just the way it is. We are not going to introduce even more special cases in the driver to workaround bugs in modules.

Who is suggesting anything about adding workarounds for module bugs? Please note that I said that I made changes to the module AND to the sqlsrv driver.

But, I am suggesting that we fix the bug in sqlsrv that prevents function calls in queries which works everywhere else but sqlsrv.

I fully agree that we should NOT introduce a change to a driver which requires changes to existing, currently compatible modules. The change that I propose does exactly the opposite, it makes sqlsrv work just like the rest of the SQL drivers (as far as I can tell). I'm testing it on my installation of a number of internal test sites right now and will submit it formally for peer review once I have verified that it causes very minimal or zero impact to existing modules (including to performance), or not if I determine otherwise.

Incidentally, you can have a more uniform implementation that doesn't require the two replacements by defining functions that call the global ones and then prepending ALL function calls with 'dbo.'. (dbo.LEN() will fail unless you do that and I suspect that replacing LENGTH has greater risk than defining dbo.LENGTH(), but that's currently just an unproven/untested opinion.)

That was either recently fixed or we have an issue for it, right?

Yes, it is issue #1208258: Schema error when creating an unsigned column with a keyword name collision. That patch was identical to the one that I resolved locally, but I am as yet reluctant to be counted as a contributor to the sqlsrv module. Soon enough I may decide to be, but not just yet. I don't even know if I'm going in the same direction as the sqlsrv driver yet. I know that I have performance requirements that may be counter to the direction of the sqlsrv module (or they may not, I simply do not yet know).

WVxNitemare’s picture

I am having this issue as well. However, I try to visit the install.php page and get a blank page. Says the website cannot display the page. Any help would be appreciated, or is there a manual way to recreate what needs to be created. Thanks,