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

fix: allow implicit cast of numbers literals to decimals on insert/select #6005

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

spena
Copy link
Member

@spena spena commented Aug 12, 2020

Description

Fixes #5285

Allows implicitly casting of numeric literals to decimals when executing an INSERT/SELECT statement. A conversion from one literal type to another is done in the SelectionUtil.buildSelectExpressions() method, which uses the ImplicitlyCastResolver to cast a supported type if necessary.

A conversion of column schema is used Instead of just allowing the schema during the validation in the EngineExecutor.validateExistingSink(). The reason is that SchemaRegistry/AVRO does not allow some type of conversions, like INT/BIGINT -> DECIMAL as they are incompatible.

The validation in the EngineExecutor.validateExistingSink()`` is required only for DECIMAL -> DECIMAL of different precision and scale. ImplicitlyCastResolver` can only convert to a different scale, but precision is based on the literal number. Without this, the validation will fail due to a different precision.

Implicitly cast is supported on the following literal types:

  • INT,BIGINT,DOUBLE -> DECIMAL
  • DECIMAL -> DECIMAL from lower to higher precision/scale

When casting any number to DECIMAL, the ImplicitlyCastResolver makes sure that the precision/scale of the number is compatible with the target decimal precision/scale. The DecimalUtil.canImplicitlyCast() is used to check that.

The reason to not allow non-literals numbers is because INT,BIGINT,DOUBLE numbers found in the topic records might have different precision than the one defined in the target decimal: i.e. DOUBLE -> DECIMAL(1,0).

Other implicitly conversions, such as INT -> BIGINT,DOUBLE, BIGINT -> DOUBLE are not possible yet as there's another refactor to do to support that. Turns out that when the INT -> BIGINT is allowed by this PR, then when the node reads the statement text from the command topic and parses it, it creates an IntegerLiteral instead of the desired LongLiteral. More refactoring is needed to support that, but this PR is fixing only #5285 for now.

Testing done

  • Added unit tests

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@spena spena requested a review from a team August 12, 2020 20:44
@spena spena force-pushed the issue_5285 branch 2 times, most recently from 4a29205 to 8edb279 Compare August 12, 2020 22:13
@spena spena changed the title fix: allow implicit cast for number literals on insert/select fix: allow implicit cast of numbers literals to decimals on insert/select Aug 13, 2020
Copy link
Member

@stevenpyzhang stevenpyzhang left a comment

Choose a reason for hiding this comment

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

LGTM

@spena spena merged commit 2bc15dd into confluentinc:master Aug 18, 2020
@spena spena deleted the issue_5285 branch August 18, 2020 15:52
sarwarbhuiyan pushed a commit to sarwarbhuiyan/ksql that referenced this pull request Sep 4, 2020
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.

INSERT INTO does not allow full range of decimal values
2 participants