Lucee icon indicating copy to clipboard operation
Lucee copied to clipboard

Catch named params left in QoQ SQL

Open bdw429s opened this issue 3 years ago • 5 comments
trafficstars

This fixes the case where named params are used in a QoQ but no params are provided to the query.

https://luceeserver.atlassian.net/browse/LDEV-4044

bdw429s avatar Sep 19 '22 21:09 bdw429s

@zspitzer @micstriit

bdw429s avatar Sep 19 '22 21:09 bdw429s

[javac] /home/runner/work/Lucee/Lucee/core/src/main/java/lucee/runtime/sql/SelectParser.java:29: error: cannot find symbol
[javac] import lucee.runtime.exp.IllegalQoQException;
[javac]                         ^
[javac]   symbol:   class IllegalQoQException
[javac]   location: package lucee.runtime.exp
[javac] /home/runner/work/Lucee/Lucee/core/src/main/java/lucee/runtime/sql/SelectParser.java:46: error: cannot find symbol
[javac] import lucee.runtime.exp.IllegalQoQException;
[javac]                         ^
[javac]   symbol:   class IllegalQoQException
[javac]   location: package lucee.runtime.exp
[javac] Note: Processing Log4j annotations
[javac] Note: No elements to process

zspitzer avatar Sep 20 '22 07:09 zspitzer

Weird, I'll check. This code is totally compiling and passing the tests here.

bdw429s avatar Sep 20 '22 16:09 bdw429s

@zspitzer Oh, I know the issue. That class is in the other pull because this pull depends on the new functionality I added in the first pull. You'll need to merge that pull first.

bdw429s avatar Sep 20 '22 16:09 bdw429s

@zspitzer When I did the work for this pull, I was working on the branch from the previous pull. I only pulled those changes over to this branch at the end when I committed them. That also means now that I think about it that after we merge the other pull, this one still won't pass until I rebase it to include those commits in this branch. I was just trying to keep the pulls organized, but wasn't thinking about it affecting the builds running in isolation. It seems every time I do QoQ tickets, they all overlap!

bdw429s avatar Sep 20 '22 16:09 bdw429s

@zspitzer Ok, now that the other pull is merged, I tried rebasing this branch to include those commits. Hopefully I did the rebase right, and the tests will pass now!

bdw429s avatar Sep 23 '22 17:09 bdw429s

Sigh, there's some random tests failing that have nothing to do with this ticket. I'm not sure if the rebase did what it was supposed to-- I hate Git sometimes, lol. @zspitzer I don't seem to have permissions, but can you re-run the failed build please. If it's still getting unrelated errors, I'll try re-creating the branch again.

bdw429s avatar Sep 23 '22 17:09 bdw429s

re-trying this pull here: https://github.com/lucee/Lucee/pull/1816

bdw429s avatar Sep 26 '22 19:09 bdw429s