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

[verifier] Add 'control_query_utf8' and 'test_query_utf8' columns. #23099

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

spershin
Copy link
Contributor

@spershin spershin commented Jun 28, 2024

Description

Add 'control_query_utf8' and 'test_query_utf8' columns to the verifier query table.

Motivation and Context

We realized that existing 'control_query' and 'test_query' columns are of latin1 format, which
results is some utf8 characters loss.
Direct conversion of the table columns is failing due to the existing data, so we have come
up with a workaround via the additional temporary columns.
At some point we can remove either the new columns or the old ones and keep the new ones.

The new columns are used if not null, otherwise the old ones are used.

Test Plan

Made a custom build and tested it on a altered table.
Works like a charm!

== NO RELEASE NOTE ==

@spershin spershin requested a review from a team as a code owner June 28, 2024 06:34
@spershin spershin requested a review from presto-oss June 28, 2024 06:34
@spershin spershin force-pushed the NewVerifierQueryColumns branch 3 times, most recently from 5bb3421 to 1216dd3 Compare June 28, 2024 18:25
@spershin spershin requested a review from rschlussel June 28, 2024 23:05
steveburnett
steveburnett previously approved these changes Jul 1, 2024
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants