mcfunley / pugsql

A HugSQL-inspired database library for Python
https://pugsql.org
Apache License 2.0
673 stars 22 forks source link

Pugsql chokes on normal comments interspersed in the sql file #62

Open ou-vlakvark opened 2 years ago

ou-vlakvark commented 2 years ago

pugsql raises the following error if there are any normal comments in the sql code (unless they are between the :name special comment and the actual sql query. ValueError: Statement must have a name.

Is this the correct behaviour? Would it not be desirable to be able to comment out sections of the sql file or add section comments etc?

mcfunley commented 2 years ago

Can you paste a query that fails?

ou-vlakvark commented 2 years ago

Hi, thanks for your quick response. This was my first time trying to use pugsql so maybe I am just doing something wrong. But here is my failing query file: start of file --------------------------------------- / pugsql seems to choke on this comment /

-- pugsql seems to choke on a single-line comment as well

-- :name greet :one / pugsql handles this comment / select 'hello' as greeting;

-- :name get_user_42 :one -- pugsql handles this comment select username from user where id = 42;

-- :name get_user_with_id :one select username from user where id = :id; end fo file-------------------------------

ou-vlakvark commented 2 years ago

Adding the following lines to the head of the above file seems to satisfy pugsql:

:name dummy_query :one select 'this stops pugsql choking on the following comments' as kludge;

ou-vlakvark commented 2 years ago

I have edited the description of the error to be more accurate

mcfunley commented 2 years ago

I see, currently pugsql needs the metadata for the query (the :name, and so on) to be in the first comment block in the file, using the -- comment syntax. There is a confusing edge here where pugsql will accept:

--- not metadata
--- :name foo
select 1;

but not:

--- not metadata

--- :name foo
select 1;

We should resolve that (probably backwards compatible with how it works now) so that it's no longer whitespace significant.

I could see also supporting a /* */ style comment, which we don't currently.

There are probably also some ways the error messages could be more helpful here.