Bug #82571 for SQL-SplitStatement: add a "keep comment but remove empty statements" option

Tue Jan 08 17:09:44 2013 MATHIAS [...] cpan.org - Ticket created

Subject:

add a "keep comment but remove empty statements" option

Hi, First of all, thank you for your module, which works absolutely fine. If possible, I have a feature request about adding an option to keep the comments in SQL code but still remove the statements that contain no code. My use case is that my code is processing user code, from which I remove empty statement (the Teradata DB output some error message on empty statement), but I would still like to keep the comments (except when they are on their own) to help users diagnose problem from the execution log. Currently, as a workaround, I am doing something along the line of: my $s = SQL::SplitStatement->new(keep_comments => 1, keep_empty_statements => 0); my $r = SQL::SplitStatement->new(keep_empty_statements => 0); my @l = grep { $r->split($_) } $s->split($sql); So this is really not a big issue, but an option to control this behaviour would be a added bonus. Thank you again for your time, Cheers, Mathias

Tue Jan 15 14:05:56 2013 emazep [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #82571] add a "keep comment but remove empty statements" option
Date:	Tue, 15 Jan 2013 20:05:37 +0100
To:	bug-SQL-SplitStatement [...] rt.cpan.org
From:	Emanuele Zeppieri <emazep [...] gmail.com>

On 2013-01-08 23:09, Mathias Kende via RT wrote: Show quoted text

> Hi, > > First of all, thank you for your module, which works absolutely fine. > > If possible, I have a feature request about adding an option to keep the > comments in SQL code but still remove the statements that contain no > code. My use case is that my code is processing user code, from which I > remove empty statement (the Teradata DB output some error message on > empty statement), but I would still like to keep the comments (except > when they are on their own) to help users diagnose problem from the > execution log.

Hi Mathias. Sorry for my late and thank you for your appreciation. The main problem with your request is that I don't know how to accommodate it in the current API: if we added an extra option, it would conflict with the current options, with the consequent need to introduce a priority (which I prefer to avoid). And if we decided to switch from a boolean option to a three-value option, such as for example: keep_comments => 0 # like now keep_comments => 1 # like now keep_comments => 2 # keep the comments only if there is also code that would be a subtle API change (people currently relying on a generic true value could see another behaviour). Show quoted text

> Currently, as a workaround, I am doing something along the line of: > > my $s = SQL::SplitStatement->new(keep_comments => 1, > keep_empty_statements => 0); > my $r = SQL::SplitStatement->new(keep_empty_statements => 0); > my @l = grep { $r->split($_) } $s->split($sql);

Nice solution! :-) Show quoted text

> So this is really not a big issue, but an option to control this > behaviour would be a added bonus.

I could introduce such feature, but I need some suggestion to avoid the problems explained above. Please let me know if you have any idea. Show quoted text

> Thank you again for your time,

Thank you to you for your contribution! -Emanuele

Tue Jan 15 14:05:56 2013 The RT System itself - Status changed from 'new' to 'open'

Mon Mar 11 18:31:33 2013 MATHIAS [...] cpan.org - Correspondence added

Show quoted text

> The main problem with your request is that I don't know how to > accommodate it in the current API: if we added an extra option, it would > conflict with the current options, with the consequent need to introduce > a priority (which I prefer to avoid).

Hi Emanuele, Thank you for your answer and sorry for not having answered to you sooner (too). I agree, that you should not change the value of the current parameters and I cannot come with a better solution than introducing an ad hoc parameter like keep_comment_statements which would default to true and would remove comment only statement if set to false. This parameter is useless if keep_comment is false (because there would be no comment statement), but it is independent of the keep_empty_statements parameter. So the only conflict which should be explained in the documentation is that if keep_comment is false and keep_comment_statement is true, then the statement containing only comments are still removed. IMHO, this is not more complex than the current situation with keep_empty_statements. Let me know if this this seems a proper solution to you ? Regards, Mathias