andialbrecht / sqlparse

A non-validating SQL parser module for Python
BSD 3-Clause "New" or "Revised" License
3.73k stars 695 forks source link

Incorrect indenting of plain update containing functions #73

Open ghost opened 12 years ago

ghost commented 12 years ago

Incorrect indenting of plain UPDATE statement containing Oracle bult-in functions (TO_TIMESTAMP).

Please see sample input and output files, along with expected result (formatted with PL/SQL Beautifier from PL/SQL Developer). As an alternative http://www.dpriver.com/pp/sqlformat.htm can be used with PL/SQL format selected.

ATTACHED FILES: https://gist.github.com/3275886

Here is the Python script that used to beautify the input SQL:

#!/usr/bin/python2
import sys
import sqlparse

if len(sys.argv) > 1:
    input = open(sys.argv[1])
else:
    input = sys.stdin

if len(sys.argv) > 2:
    output = open(sys.argv[2], 'w')
else:
    output = sys.stdout

pretty_sql = sqlparse.format(input.read(), reindent=True)
output.write(pretty_sql)
piranna commented 12 years ago

Thanks for the result and the expected output, they will be very useful :-) Unluckily i don't undestand what did you wanted to achieve. Were you trying just to format the query or did you used some filters? Would you be able to give us a little test program to see what you were trying to do? :-)

ghost commented 12 years ago

Sure Jesús, I have updated the issue. I was only trying to beautify the sql code, nothing complex. Thank you for your time and effort spent on sqlparse - it is a really useful tool. Cheers, Vio

On 6 August 2012 22:18, Jesús Leganés Combarro < reply@reply.github.com

wrote:

Thanks for the result and the expected output, they will be very useful :-) Unluckily i don't undestand what did you wanted to achieve. Were you trying just to format the query or did you used some filters? Would you be able to give us a little test program to see what you were trying to do? :-)


Reply to this email directly or view it on GitHub: https://github.com/andialbrecht/sqlparse/issues/73#issuecomment-7532767

piranna commented 12 years ago

Thank you, now it's more clear :-D I'll create a unittest and start looking at it :-)

ghost commented 12 years ago

Thank you so much! Cheers and have a great day! ) Vio

Jesús Leganés Combarro notifications@github.com wrote:

Thank you, now it's more clear :-D I'll create a unittest and start looking at it :-)


Reply to this email directly or view it on GitHub: https://github.com/andialbrecht/sqlparse/issues/73#issuecomment-7539665

andialbrecht commented 12 years ago

hm, we see a lot of issues regarding functions in identifier lists and IIRC that worked at some point. I try to be optimistic and target this issue for the next milestone :)

ghost commented 12 years ago

if it is a regression then maybe it will be even easier to fix ) thanks for looking into this

Andi Albrecht notifications@github.com wrote:

hm, we see a lot of issues regarding functions in identifier lists and IIRC that worked at some point. I try to be optimistic and target this issue for the next milestone :)


Reply to this email directly or view it on GitHub: https://github.com/andialbrecht/sqlparse/issues/73#issuecomment-7544359

piranna commented 12 years ago

Ok, i have created a unittest on branch issue_73. In any case, i see several problems here relating to what generates sqlparse and the output you expect:

Are you asking for just the fields indentation or the full output? First one is a bug, others are improvements and should be on independent issues... By the way i'll split the tests cases to make it easier to deploy.

andialbrecht commented 12 years ago

Let's just do the fields alignment here. Additional formatting rules is a totally different (and more complex) topic IMO.

On Tue, Aug 7, 2012 at 12:21 PM, Jesús Leganés Combarro notifications@github.com wrote:

Ok, i have created a unittest on branch issue_73. In any case, i see several problems here relating to what generates sqlparse and the output you expect:

fields indentation. No doubt, it's a bug keywords alignement. Currently they are put at begining of new lines, if you want that format align then on the space blank it needs some more work (althought some of the work done with fields keywords indentation would be reused...) fields alignement. Same as with keywords

Are you asking for just the fields indentation or the full output? First one is a bug, others are improvements and should be on independent issues... By the way i'll split the tests cases to make it easier to deploy.

— Reply to this email directly or view it on GitHub.

piranna commented 12 years ago

I have check it with the other future milestones and the problem is still there. I have splitted the topic on different "steps" and just how the formatter is working now, there are differences regarding to trying to fill the lines with all the fields that are possible instead of having one field per line and regarding whitespaces around equal sign. I don't remember if there are options for this ones, but if don't they would be useful and easy to implement...

- set EXTERNALBONUSAMOUNT=0.0, EXTERNALBONUS=0,
?                             -----------------
+ set EXTERNALBONUSAMOUNT = 0.0,
?                        + +
andialbrecht commented 11 years ago

I'd like to move this issue to 0.2.0. In my previous comments I've missed that this is an UPDATE statement and we have currently some issues formatting anything else than SELECT :(