Skip to content

SQL: Add range checks to interval multiplication operation #83478

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 7, 2022
Merged

SQL: Add range checks to interval multiplication operation #83478

merged 3 commits into from
Feb 7, 2022

Conversation

@bpintea
Copy link
Contributor

@bpintea bpintea commented Feb 3, 2022

This adds range checks on the multiplication operation of intervals with integers.

Fixes #83336

@bpintea
Add overflow check to interval multiplications
edbedfa 
This adds checks on the multiplication operation on intervals (with
integer).
@elasticsearchmachine
Copy link
Collaborator

Hi @bpintea, I've created a changelog YAML for you.

@bpintea
Style fix
c0e677e 
Remove redundant import.
@bpintea bpintea marked this pull request as ready for review February 3, 2022 20:16
@elasticmachine elasticmachine added the Team:QL (Deprecated) Meta label for query languages team label Feb 3, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-ql (Team:QL)

costin
costin approved these changes Feb 4, 2022
View reviewed changes
Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM

Luegg
Luegg approved these changes Feb 4, 2022
View reviewed changes
Copy link
Contributor

@Luegg Luegg left a comment

Choose a reason for hiding this comment

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

Lgtm, but I would consider adding some examples demonstrating interval multiplications to datetime-interval.csv-spec.

@bpintea
Copy link
Contributor Author

bpintea commented Feb 4, 2022

I would consider adding some examples demonstrating interval multiplications to datetime-interval.csv-spec

@Luegg, additional to the ones starting on (currently) line 171?

@Luegg
Copy link
Contributor

Luegg commented Feb 4, 2022

I would consider adding some examples demonstrating interval multiplications to datetime-interval.csv-spec

@Luegg, additional to the ones starting on (currently) line 171?

Uh, I missed these, sorry. Never mind then, looks even better to me now :)

luigidellaquila
luigidellaquila approved these changes Feb 7, 2022
View reviewed changes
Copy link
Contributor

@luigidellaquila luigidellaquila left a comment

Choose a reason for hiding this comment

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

LGTM

@bpintea bpintea merged commit 7c6a2a6 into elastic:master Feb 7, 2022
@bpintea bpintea deleted the fix/interval_mul_overflow branch February 7, 2022 09:05
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Feb 7, 2022
@weizijun
Merge branch 'upstream/master' into rollover-add-max_shard_docs
ae9786a 
* upstream/master:
  [DOCS] Switch xrefs to external links (elastic#83590)
  [DOCS] 'features' flag added in elastic#83083 (elastic#83452)
  Rename ChangePolicyforIndexIT to ChangePolicyForIndexIT (elastic#83569)
  Fixing random_sampler tests (elastic#83549)
  Upgrade Checkstyle to 9.3 (elastic#83314)
  Make improvements to the release notes generator (elastic#83525)
  Cleanup DataTierAllocationDecider (elastic#83572)
  Upgrade jANSI dependency to 2.4.0 (elastic#83566)
  Speed up Name Collision Check in Metadata.Builder (elastic#83340)
  SQL: Add range checks to interval multiplication operation (elastic#83478)
  Remove DiscoveryNodes#getAllNodes (elastic#83538)
  Make RoutingNodes behave like a collection (elastic#83540)
  Remove Unused CS Listener from SecurityServerTransportInterceptor (elastic#83556)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@costin costin costin approved these changes

@luigidellaquila luigidellaquila luigidellaquila approved these changes

@astefan astefan Awaiting requested review from astefan

+1 more reviewer

@Luegg Luegg Luegg approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Analytics/SQL SQL querying >bug Team:QL (Deprecated) Meta label for query languages team v8.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SQL: bounds of interval multiplication are not checked

6 participants

@bpintea @elasticsearchmachine @elasticmachine @Luegg @costin @luigidellaquila