Skip to content

Conversation

@gianm
Copy link
Contributor

@gianm gianm commented Sep 19, 2025

Building on #18521, this patch makes virtual column specialization recursive and adds specializations for two additional JSON functions. The result is that chains of JSON_OBJECT, JSON_MERGE, and JSON_VALUE can preserve lazy evaluation, index usage, dictionary usage, etc.

There is a change to VirtualColumnCreator that can affect extensions that add SQL operators. To allow the creator to access rewritten arguments, a "DruidExpression self" parameter is added. The "String expression" is no longer needed so it is removed.

Building on apache#18521, this patch makes virtual column specialization
recursive. It also now happens immediately on calling
getOrCreateVirtualColumnForExpression.

Specializations are added for JSON_OBJECT and JSON_MERGE. Now, chains of
JSON_MERGE, JSON_OBJECT, and JSON_VALUE can preserve lazy evaluation,
index usage, dictionary usage, etc.

There is a change to VirtualColumnCreator that can affect extensions
that add SQL operators. To allow the creator to access rewritten
arguments, a "DruidExpression self" parameter is added.
The "String expression" is no longer needed so it is removed.
@gianm gianm force-pushed the sql-specialize-recursively branch from e5b743e to d786c7e Compare September 20, 2025 04:28
Comment on lines -738 to -739
final boolean forceExpressionVirtualColumns =
plannerContext.getPlannerConfig().isForceExpressionVirtualColumns();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like this flag is no longer honored, should we push the config into the virtual column registry?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did do that, it's pushed in and if true then VirtualColumnRegistry#specialize exits without doing anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah so it is 😅

@gianm gianm merged commit 54bb2fd into apache:master Oct 16, 2025
91 of 93 checks passed
@gianm gianm deleted the sql-specialize-recursively branch October 16, 2025 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants