LucidDB / luciddb

DEFUNCT: See README
https://github.com/LucidDB/luciddb
Apache License 2.0
52 stars 24 forks source link

[FRG-149] SqlPrettyWriter.setSelectListItemsOnSeparateLines is false advertising #723

Closed dynamobi-build closed 12 years ago

dynamobi-build commented 12 years ago

[reporter="jvs", created="Thu, 8 Jun 2006 18:08:13 -0500 (GMT-05:00)"] You can verify this by looking at SqlPrettyWriterTest.ref.xml, which is currently checked in with incorrect results.

The bug is due to the fact that SqlNodeList.unparse creates a new Frame, ignoring the correctly configured Frame which SqlSelectOperator set up for it.

I don't want to disturb SqlNodeList, which is used many place, so I'll probably put the fix in SqlSelectOperator, making it call a newly public version of SqlNodeList.commaList directly.

What about the ORDER BY and GROUP BY lists? They have the same issue. I'm not sure if they're actually supposed to be following this setting or not because setSelectListItemsOnSeparateLines doesn't sound like it should affect them. But the code inside of SqlPrettyWriter seems to want them to follow the select list behavior.

dynamobi-build commented 12 years ago

[author="jhyde", created="Fri, 9 Jun 2006 08:36:04 -0500 (GMT-05:00)"] Yes, this option should break select lists and order by and group by lists too.

Note that I'm refactoring SqlNodeList.accept(SqlVisitor) right now, as part of my order by fix.

dynamobi-build commented 12 years ago

[author="jvs", created="Sat, 10 Jun 2006 14:41:07 -0500 (GMT-05:00)"] Fixed in eigenchange 6856. Note that I made it come out Broadbase-style:

SELECT
    a,
    b,
    ...

rather than with the first item on the same line as the open:

SELECT a
    b,
    ...

Because that's just ugly.