Skip to content

VARIANT code cleanups and improvements#28939

Merged
wendigo merged 11 commits intomasterfrom
user/serafin/variant-fixes-and-improvements
Apr 1, 2026
Merged

VARIANT code cleanups and improvements#28939
wendigo merged 11 commits intomasterfrom
user/serafin/variant-fixes-and-improvements

Conversation

@wendigo
Copy link
Copy Markdown
Contributor

@wendigo wendigo commented Mar 31, 2026

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Mar 31, 2026
@wendigo wendigo requested a review from findepi March 31, 2026 12:15
@wendigo wendigo force-pushed the user/serafin/variant-fixes-and-improvements branch from f47798c to b2fa813 Compare March 31, 2026 12:59
@wendigo wendigo requested a review from dain March 31, 2026 12:59
@wendigo wendigo force-pushed the user/serafin/variant-fixes-and-improvements branch from b2fa813 to e2cc8f9 Compare March 31, 2026 14:08
@wendigo wendigo requested a review from losipiuk March 31, 2026 14:57
Copy link
Copy Markdown
Member

@dain dain left a comment

Choose a reason for hiding this comment

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

Other than the last commit this looks good

Comment thread core/trino-spi/src/main/java/io/trino/spi/variant/Variant.java
generator.copyCurrentStructure(parser);
}
return writer.toString();
return JSON_MAPPER.writeValueAsString(JSON_MAPPER.readTree(parser));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is a lot more expensive. The old code, had a parser copying tokens directly into a generator, and I think the new code creates a JsonNode, which is a big object graph.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Partially reverted. PTAL

@wendigo wendigo force-pushed the user/serafin/variant-fixes-and-improvements branch from e2cc8f9 to 1efa7cb Compare March 31, 2026 19:01
@wendigo
Copy link
Copy Markdown
Contributor Author

wendigo commented Mar 31, 2026

@dain PTAL

@wendigo wendigo merged commit d2ef46d into master Apr 1, 2026
115 checks passed
@wendigo wendigo deleted the user/serafin/variant-fixes-and-improvements branch April 1, 2026 05:20
@github-actions github-actions bot added this to the 481 milestone Apr 1, 2026
@ebyhr ebyhr mentioned this pull request Apr 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants