Thread: BUG #8611: ECPG: unclosed comment "/*"
The following bug has been logged on the website:
Bug reference: 8611
Logged by: Alexei Savchik
Email address: alexsav23@gmail.com
PostgreSQL version: 9.3.1
Operating system: Windows 8 x64
Description:
pre-compiler don't work if we have unclosed comment:
int func(void)
{
/*
/* Build exec sql statement
*/
EXEC SQL SELECT 1;
return 0;
}
d:\>"c:\Program Files\PostgreSQL\9.3\bin\ecpg.exe" 1.pgc
1.pgc: 9: ERROR: incomplete comment / *
(message translate from Russian)
it is not critical, but it difficult migrate from other DB (for example from
SybaseASE pre-compiler)
On Wed, Nov 20, 2013 at 11:18:18AM +0000, alexsav23@gmail.com wrote: > pre-compiler don't work if we have unclosed comment: Right, this comes from ecpg using the same rules for C-style commands for SQL code and C code. However, this is not correct, I agree. SQL allows for nested comments while C does not. This needs fixing. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
Michael Meskes <meskes@postgresql.org> writes:
> On Wed, Nov 20, 2013 at 11:18:18AM +0000, alexsav23@gmail.com wrote:
>> pre-compiler don't work if we have unclosed comment:
> Right, this comes from ecpg using the same rules for C-style commands for SQL
> code and C code. However, this is not correct, I agree. SQL allows for nested
> comments while C does not. This needs fixing.
I don't see any way to "fix" it --- the best you can do is have a
different set of users complaining, because C and SQL aren't consistent
about this, and it's not obvious which convention should be followed
where.
Perhaps we should just document that you should avoid nested comments
in ecpg source code.
regards, tom lane
On Fri, Nov 22, 2013 at 03:19:10PM -0500, Tom Lane wrote: > I don't see any way to "fix" it --- the best you can do is have a > different set of users complaining, because C and SQL aren't consistent > about this, and it's not obvious which convention should be followed > where. Well, I think we can in ecpg as ecpg knows whether it's parsing C code or SQL code. Alexei, could you please try the attached patch? IT's already in HEAD but I'd prefer to have you run your sources through the changed ecpg before backporting the patch. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
Attachment
Michael Meskes <meskes@postgresql.org> writes:
> On Fri, Nov 22, 2013 at 03:19:10PM -0500, Tom Lane wrote:
>> I don't see any way to "fix" it --- the best you can do is have a
>> different set of users complaining, because C and SQL aren't consistent
>> about this, and it's not obvious which convention should be followed
>> where.
> Well, I think we can in ecpg as ecpg knows whether it's parsing C code or SQL
> code.
Does it? In the example at hand, the questionable comment was after some
C code and before some SQL code. I'd say it's just about exactly 50-50
odds as to whether the user will think that C or SQL conventions should
apply at that spot. I remain of the opinion that changing behavior here
will annoy more people than it will help.
In any case, you won't make any friends at all unless you can document
exactly what the behavior is (and I'm disappointed to see that your
behavior-changing commit did not touch the documentation). There might
be some reason to change if the new behavior is more easily explainable
than the old.
regards, tom lane
Michael, Tom,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Michael Meskes <meskes@postgresql.org> writes:
> > Well, I think we can in ecpg as ecpg knows whether it's parsing C code =
or SQL
> > code.=20
I tend to agree with Tom here but I feel like I'm missing something-
will this only make things "work" which would fail previously..? We
certainly don't want to risk breaking things for existing users, but
perhaps it's not possible to have existing code which works and this is
only 'loosening' of what we accept.
Thanks,
Stephen
On Sun, Nov 24, 2013 at 11:09:57AM -0500, Tom Lane wrote: > Does it? In the example at hand, the questionable comment was after some > C code and before some SQL code. I'd say it's just about exactly 50-50 Yes, it does. Everything between EXEC SQL and the semicolon is SQL by definition, the rest is C. > odds as to whether the user will think that C or SQL conventions should > apply at that spot. I remain of the opinion that changing behavior here > will annoy more people than it will help. I don't agree. Please keep in mind that ecpg can spit out illegal C-code as it is now. It accepts and outputs nested comments that the C compiler obviously does not accept. Or in other words, I cannot see that people have code that's broken by the change I made. BTW I did notice that the patch is incomplete, though. I need to add another another check or the nested comments still get put into the C file. > In any case, you won't make any friends at all unless you can document > exactly what the behavior is (and I'm disappointed to see that your > behavior-changing commit did not touch the documentation). There might > be some reason to change if the new behavior is more easily explainable > than the old. I'm absolutely willing to check and update the docs, but didn't so far because I see this as a bug that got fixed and not a new or changed feature. So far there is nothing about comments in the docs at all AFAICT. I'd expect people to expect standard C behaviour. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
On Sun, Nov 24, 2013 at 09:53:29PM -0500, Stephen Frost wrote: > I tend to agree with Tom here but I feel like I'm missing something- > will this only make things "work" which would fail previously..? We Yes, that's the way I see it. > certainly don't want to risk breaking things for existing users, but > perhaps it's not possible to have existing code which works and this is > only 'loosening' of what we accept. We're not exactly loosening but we aligning with what the C compiler accepts, if that makes any sense. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
Michael Meskes <meskes@postgresql.org> writes:
> On Sun, Nov 24, 2013 at 09:53:29PM -0500, Stephen Frost wrote:
>> I tend to agree with Tom here but I feel like I'm missing something-
>> will this only make things "work" which would fail previously..? We
>> certainly don't want to risk breaking things for existing users, but
>> perhaps it's not possible to have existing code which works and this is
>> only 'loosening' of what we accept.
> We're not exactly loosening but we aligning with what the C compiler accepts,
> if that makes any sense.
What I was concerned about was the fear that previously-working ecpg
programs would be broken by this change. I think what you're saying
is that any such program would have failed anyhow at the compilation
stage --- is that right? If so I withdraw the complaint.
regards, tom lane
On Mon, Nov 25, 2013 at 09:34:00AM -0500, Tom Lane wrote: > What I was concerned about was the fear that previously-working ecpg > programs would be broken by this change. I think what you're saying > is that any such program would have failed anyhow at the compilation > stage --- is that right? If so I withdraw the complaint. Yes, that is indeed correct. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL
---------- Forwarded message ----------
From: Alexei Savchik <alexsav23@gmail.com>
Date: Sun, Nov 24, 2013 at 3:01 AM
Subject: Re: [BUGS] BUG #8611: ECPG: unclosed comment "/*"
To: Tom Lane <tgl@sss.pgh.pa.us>
Hi!
It will be great if fix it.
I developer from Ispirer and I work on automatically migrate from CPRE
(Sybase ASE) to ECPG.
We solved that problem, adding space to nested comments ("/*" -> "/ *") in
migration process.
We have one more issue (I reported it).
> Subject: BUG #8608: ECPG: sizeof() in EXEC SQL DECLARE SECTION
> The following bug has been logged on the website:
>
> Bug reference: 8608
> Logged by: Alexei Savchik
> Email address: alexsav23@gmail.com
> PostgreSQL version: 9.3.1
> Operating system: Windows 8 x64
> Description:
>
> void MyFunc( MyClass*unit_address )
> {
> EXEC SQL BEGIN DECLARE SECTION;
> unsigned char var1[sizeof(MyClass)];
> EXEC SQL END DECLARE SECTION;
> }
>
>
> d:\>"c:\Program Files\PostgreSQL\9.3\bin\ecpg.exe"-o 1.c 1.pgc
> 1.pgc: 4: ERROR: syntax error at or near "("
> error deleting output file "1.c"
>
>
>
http://stackoverflow.com/questions/20049432/ecpg-sizeof-in-exec-sql-declare-section
If you have solution, advice or you can to fix it, will by great!
Thank you for your answer!
Best regards
Alexei Savchik
Applications Migration Consultant from Ispirer Systems
http://www.ispirer.com/