tracker issue | What iT iS dESign studios

Title:

Please parse out /**/ SQL style comments in Query of Queries.

| View in Tracker

Status/Resolution/Reason: Closed/Won't Fix/NotWorthEffort

Reporter/Name(from Bugbase): Chris Phillips / Chris Phillips (cf_chris)

Created: 02/05/2013

Components: Database, Query-of-Query(IMQ)

Versions: 10.0

Failure Type: Enhancement Request

Found In Build/Fixed In Build: Final /

Priority/Frequency: Trivial / Unknown

Locale/System: English / Win 2008 Server R2 86 bit

Vote Count: 2

If you put a /**/ (slash star) comment in a query of queries (dbtype=query), you will get a syntax error. This is not huge deal. But, it has caused me to lose time in troubleshooting more than once.

It seems like it would be really easy to just parse that style of comment out of the SQL text before running the Q of Q.

----------------------------- Additional Watson Details -----------------------------

Watson Bug ID:	3496254

External Customer Info:
External Company:  
External Customer Name: cf_chris
External Customer Email:  
External Test Config: My Hardware and Environment details:

Windows 7

ColdFusion: 10,0,7,283649

Update Level: /I:/ColdFusion/InstanceDP1/lib/updates/chf10000007.jar

Attachments:

October 09, 2013 00:00:00: 1_index.cfm

Comments:

/**/ and -- are ANSI standards, I believe, so should be respected.

-- 

Adam

Vote by External U.

16388 | September 29, 2013 04:21:31 AM GMT

It is very unlikely to have comments inside query of query. Usually one would use comments in the sql scripts which is not the case here. We haven't had any request for this in last 10 years for this which makes it a very low priority bug and hence it is not worth the effort required to change the SQL parser.

Comment by Rupesh K.

16380 | October 08, 2013 07:04:27 AM GMT

This would take like 10 minutes to accomplish with a REGEX. If I provide the REGEX will you consider doing it. It's so damn easy. Why not do it?

Comment by External U.

16381 | October 08, 2013 12:42:34 PM GMT

I think that this is worth the effort.

Vote by External U.

16389 | October 08, 2013 12:48:48 PM GMT

It is not as easy as REGEX. It is a JavaCC based SQL parser. Unless it is absolutely critical, which I don't see in this case, there is not much value in changing the parser.

Comment by Rupesh K.

16382 | October 08, 2013 12:54:17 PM GMT

It's obviously not critical. But, I have reworked queries to QofQ in the past and got bit because they had comments.

I figured it was a simple matter of you stripping them out *before* you hand the string of SQL to the JavaCC SQL Parser.

Comment by External U.

16383 | October 08, 2013 01:02:58 PM GMT

OK, all things being equal: I think this should "just work". But given there seems to be some sort of complexity to it.... I retract my vote. It's no biggie to just use CFML comments instead, surely? The code WILL be in a CFML file after all, yes?

I think you're reading the situation wrong, Chris... SQL-style comments are the remit of the SQL parser to remove. *CFML* comment I could understand being extracted before the SQL string was passed to the SQL engine.

In a moment the date and time of which should be noted down... I agree with Rupesh here.

-- 
Adam

Comment by External U.

16384 | October 08, 2013 05:00:52 PM GMT

Adam, I understand what you're saying about it being the responsibility of the SQL engine to deal with the comments.

Rupesh, is the "JavaCC SQL Parser" you mention a third party product? Can I submit a ticket for their SQL engine to handle ANSI SQL comments?

Again, it's not a big deal. But, if a 5 minute fix can make something easier to use and prevent bugs. Why not?

Comment by External U.

16385 | October 08, 2013 05:53:04 PM GMT

Rupesh, I'm going to attach a file that runs a regular expression on some SQL text and then runs it as a QofQ. This is how simple it would be for you to do this. If you choose not to use it, then fine.

ReReplaceNoCase(sql, "/\*.*?\*/", " ", "all")

Run against:
	SELECT
		ID, /* ID is an integer */
		Type, /* Valid types are "Widget" or "Gadget */
		Name/*, I DON'T NEED THIS COLUMN
		isActive*/
	FROM
		Qry
	WHERE
		/* Only select *ACTIVE* items */
		isActive = 1

Yields:
	SELECT
		ID,  
		Type,  
		Name 
	FROM
		Qry
	WHERE
		 
		isActive = 1

Comment by External U.

16386 | October 08, 2013 05:56:39 PM GMT

That regex has no letters in it, so the NoCase is just noise.

It's also not very efficient - a better way is /\*(?:[^*]+|\*(?!/))*\*/

(This applies to most situations where .*? is used.)

But the crux of the issue is that it doesn't know whether you're in a string, and thus fails for examples like:

	SELECT '*\O/*' as emoticon FROM query WHERE emoticon = '*/O/*'

	SELECT content + '/* wibble */' AS content FROM scripts WHERE type = 'something'

A regex to cope with those could be written, but it'd be ugly and would slow down QoQ for everyone.

HOWEVER whilst it's not just a case of adding a regex, that doesn't mean it's not easy to do.

Modifying a JavaCC SQL grammar to support both forms of ANSI SQL commenting is simple enough, and wont slow things down, and it's not going to break anyone's existing code.

So I don't see why this has been marked "NeverFix". :/

Comment by External U.

16387 | October 09, 2013 04:32:10 PM GMT

tracker issue : CF-3496254

Please parse out /**/ SQL style comments in Query of Queries.