Disallow duplicate CTE aliases#13664
Conversation
yashmayya
added
enhancement
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #13664 +/- ##
============================================
+ Coverage 61.75% 61.92% +0.17%
- Complexity 207 1321 +1114
============================================
Files 2436 2554 +118
Lines 133233 140492 +7259
Branches 20636 21855 +1219
============================================
+ Hits 82274 86997 +4723
- Misses 44911 46848 +1937
- Partials 6048 6647 +599
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
yashmayya
force-pushed
the
duplicate-cte-alias
branch
from
d2cd08c to
5fccf83
Compare
| for (SqlNode sqlWithItem : with.withList) { | ||
| for (String name : ((SqlWithItem) sqlWithItem).name.names) { | ||
| if (withNames.contains(name)) { | ||
| throw new RuntimeException("Duplicate alias in WITH: '" + name + "'"); |
There was a problem hiding this comment.
Can we also provide lines? Given we are at SqlNode here, we should be able to access them
| assertEquals(tableNames.get(0), "a"); | ||
| } | ||
|
|
||
| @Test(expectedExceptions = RuntimeException.class) |
There was a problem hiding this comment.
can we also verify the message is what we expect?
There was a problem hiding this comment.
I'd initially tried that with expectedExceptionsMessageRegExp but looks like that only checks the top level exception message (and not nested exceptions) which in our case is the RuntimeException with Error composing query plan for:... from here. I'm not sure why we use TestNG and not JUnit which is a lot more feature rich 🤷
I updated the test to use a more old school style of catching the expected exception and making assertions in the catch block.
Edit: Never mind, looks like TestNG also supports the assertThrows / expectThrows style.
3 participants
WITH tmp AS (SELECT * FROM a LIMIT 1), tmp AS (SELECT * FROM a LIMIT 2) SELECT * FROM tmpdoesn't return an error and runs fine.