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

Support hex string literal #6767

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Conversation

ShiKaiWi
Copy link
Member

Which issue does this PR close?

Closes #6764 .

Rationale for this change

Datafusion supports the binary data type, but there is no way to insert binary data or query with binary data in a predicate through a sql. And this is caused by missing conversion from sqlparser::ast::Value::HexStringLiteral to datafusion_expr::Expr::Literal(ScalarValue::Binary).

What changes are included in this PR?

Support the conversion from sqlparser::ast::Value::HexStringLiteral to datafusion_expr::Expr::Literal(ScalarValue::Binary).

Are these changes tested?

Add a unit test for hex string literal decoding.

Are there any user-facing changes?

Now, the sql including hex string literal, e.g. X'FF01', can be processed and treated as a binary data by datafusion.

@github-actions github-actions bot added the sql SQL Planner label Jun 26, 2023
@ShiKaiWi ShiKaiWi force-pushed the feat-support-hex-literal branch from c29da34 to 6fe9d99 Compare June 26, 2023 09:51
@jackwener jackwener self-requested a review June 26, 2023 11:29
@ShiKaiWi ShiKaiWi force-pushed the feat-support-hex-literal branch from fb7a983 to ba495dc Compare June 26, 2023 11:39
Copy link
Member

@jackwener jackwener left a comment

Choose a reason for hiding this comment

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

Thanks @ShiKaiWi

@jackwener
Copy link
Member

Can you add a sqllogicaltest for this case? @ShiKaiWi
You can see directory datafusion/core/tests/sqllogictests

Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @ShiKaiWi -- this looks great

The only thing I think it needs are some end to end tests in .slt -- I made some in #6770 which we can merge after this one.

@alamb
Copy link
Contributor

alamb commented Jun 27, 2023

Something about this PR appears to be problematic:

Screenshot 2023-06-27 at 6 11 11 AM

@tustvold is that what you were trying to fix?

@tustvold
Copy link
Contributor

tustvold commented Jun 27, 2023

@tustvold is that what you were trying to fix?

Yes, I accidentally force pushed main, it should be reverted now and #6775 should prevent this in future, but the PR dialog appears to be a tad confused by this. Will try closing and reopening

@tustvold tustvold closed this Jun 27, 2023
@tustvold tustvold reopened this Jun 27, 2023
@tustvold tustvold changed the base branch from main to backup-main June 27, 2023 11:09
@tustvold tustvold changed the base branch from backup-main to main June 27, 2023 11:09
@alamb alamb merged commit 4f2933f into apache:main Jun 27, 2023
@alamb
Copy link
Contributor

alamb commented Jun 27, 2023

#6770 is now ready for review too (with some basic .slt tests)

@ShiKaiWi ShiKaiWi deleted the feat-support-hex-literal branch June 28, 2023 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sql SQL Planner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support binary data type literals
4 participants