Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(sql): fix subqueries returning a varchar column #4492

Merged
merged 13 commits into from
May 16, 2024

Conversation

jerrinot
Copy link
Contributor

@jerrinot jerrinot commented May 10, 2024

Some SQLs with subqueries returning a varchar column were broken.

Example:

select * from outer where sym in (select varchar from sub where symbol = 'baz'),

The IN operator accepts a subquery returning a varchar column and this was broken.

Implementation note:
This bug was caused by some (delegating) RecordCursorFactories that expected inner Record to implicitly cast VARCHAR to STRING or SYMBOL. While Function performs such casting, Record does not. To complicate matters further, VirtualRecord appears to perform casting because it merely delegates to a function, which then handles the casting. This PR updates the affected CursorRecordFactories to explicitly convert VARCHAR columns to UTF-16 when necessary.

@jerrinot jerrinot added Bug Incorrect or unexpected behavior SQL Issues or changes relating to SQL execution Java Improvements that update Java code labels May 10, 2024
@jerrinot jerrinot marked this pull request as draft May 10, 2024 08:42
@jerrinot
Copy link
Contributor Author

there are more cases to fix

@jerrinot jerrinot marked this pull request as ready for review May 10, 2024 12:57
@jerrinot
Copy link
Contributor Author

it's ... not great. but doing this properly would require major changes. hopefully it will get simpler again once we start storing symbols as UTF8.

@jerrinot jerrinot marked this pull request as draft May 11, 2024 10:55
@jerrinot jerrinot marked this pull request as ready for review May 13, 2024 08:38
@ideoma ideoma self-requested a review May 14, 2024 17:39
@bluestreak01
Copy link
Member

@jerrinot could you rephrase PR title and add description. The existing title is very cryptic.

@jerrinot jerrinot changed the title chore(sql): key-ed subqueries with varchar columns fixed chore(sql): fix subqueries returning a varchar column May 16, 2024
@ideoma
Copy link
Collaborator

ideoma commented May 16, 2024

[PR Coverage check]

😍 pass : 35 / 39 (89.74%)

file detail

path covered line new line coverage
🔵 io/questdb/cairo/sql/Record.java 7 11 63.64%
🔵 io/questdb/griffin/engine/table/FilterOnSubQueryRecordCursorFactory.java 2 2 100.00%
🔵 io/questdb/griffin/engine/table/LatestBySubQueryRecordCursorFactory.java 2 2 100.00%
🔵 io/questdb/griffin/engine/functions/bool/InSymbolCursorFunctionFactory.java 12 12 100.00%
🔵 io/questdb/griffin/SqlCodeGenerator.java 12 12 100.00%

@bluestreak01 bluestreak01 merged commit 9b39249 into master May 16, 2024
24 checks passed
@bluestreak01 bluestreak01 deleted the jh_subquery_varchar branch May 16, 2024 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Incorrect or unexpected behavior Java Improvements that update Java code SQL Issues or changes relating to SQL execution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants