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.

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

remove extractionFn from equality, null, and range filters #14612

Merged
merged 5 commits into from
Jul 19, 2023
Merged

remove extractionFn from equality, null, and range filters #14612

merged 5 commits into from
Jul 19, 2023

Conversation

clintropolis
Copy link
Member

@clintropolis clintropolis commented Jul 19, 2023 •
edited
Loading

Description

Follow up to #14542 with a few modifications and fixes

changes:

  • EqualityFilter, NullFilter, and RangeFilter no longer support extractionFn
  • SQL planner will use ExpressionFilter in the small number of cases where an extractionFn would have been used if sqlUseBoundsAndSelectors is set to false instead of equality/null/range filters
  • fix bugs and add tests with serde, equals, and cache key for null, equality, and range filters

This PR has:

  • been self-reviewed.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

@clintropolis clintropolis added this to the 27.0 milestone Jul 19, 2023
@clintropolis
remove extractionFn from equality, null, and range filters
95fab41 
changes:
* EqualityFilter, NullFilter, and RangeFilter no longer support extractionFn
* SQL planner will use ExpressionFilter in the small number of cases where an extractionFn would have been used if sqlUseBoundsAndSelectors is set to false instead of equality/null/range filters
* fix bugs and add tests with serde, equals, and cache key for null, equality, and range filters
@clintropolis clintropolis force-pushed the remove-extractionFn-for-new-filters branch from 313e81f to 95fab41 Compare July 19, 2023 06:43
gianm
gianm approved these changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

LGTM. Glad to see things moving away from extractionFn and towards expressions.

@gianm gianm merged commit 68fd221 into apache:master Jul 19, 2023
74 checks passed
@clintropolis clintropolis deleted the remove-extractionFn-for-new-filters branch July 19, 2023 18:27
gianm added a commit to gianm/druid that referenced this pull request Jul 19, 2023
@gianm
Simplify bounds/range vs selectors/equality logic in SQL planning.
664dfdf 
1) Consolidate duplicate code related to Expressions#buildTimeFloorFilter.

2) Cleaner logic in Expressions#toSimpleLeafFilter: choose bounds vs range
   filter based solely on plannerContext.isUseBoundsAndSelectors, not also
   considering rhs kind. Use parsed rhs in both paths (except for numerics
   in the bound path).

3) Fix ArrayContains, ArrayOverlap to avoid equality filters when there is
   an extractionFn present. Fixes a bug introduced in apache#14612.
@gianm gianm mentioned this pull request Jul 19, 2023
Merged
@clintropolis clintropolis mentioned this pull request Jul 19, 2023
Merged
clintropolis added a commit to clintropolis/druid that referenced this pull request Jul 19, 2023
@clintropolis
remove extractionFn from equality, null, and range filters (apache#14612
b0c0685 
)

* remove extractionFn from equality, null, and range filters
changes:
* EqualityFilter, NullFilter, and RangeFilter no longer support extractionFn
* SQL planner will use ExpressionFilter in the small number of cases where an extractionFn would have been used if sqlUseBoundsAndSelectors is set to false instead of equality/null/range filters
* fix bugs and add tests with serde, equals, and cache key for null, equality, and range filters

* test coverage fixes bugs

* adjust

* adjust again

* so persnickety
clintropolis added a commit that referenced this pull request Jul 19, 2023
@clintropolis
remove extractionFn from equality, null, and range filters (#14612) (#…
8a27317 
…14621)

* remove extractionFn from equality, null, and range filters
changes:
* EqualityFilter, NullFilter, and RangeFilter no longer support extractionFn
* SQL planner will use ExpressionFilter in the small number of cases where an extractionFn would have been used if sqlUseBoundsAndSelectors is set to false instead of equality/null/range filters
* fix bugs and add tests with serde, equals, and cache key for null, equality, and range filters

* test coverage fixes bugs

* adjust

* adjust again

* so persnickety
gianm added a commit that referenced this pull request Jul 20, 2023
@gianm
Simplify bounds/range vs selectors/equality logic in SQL planning. (#…
c2e6758 
…14619)

* Simplify bounds/range vs selectors/equality logic in SQL planning.

1) Consolidate duplicate code related to Expressions#buildTimeFloorFilter.

2) Cleaner logic in Expressions#toSimpleLeafFilter: choose bounds vs range
   filter based solely on plannerContext.isUseBoundsAndSelectors, not also
   considering rhs kind. Use parsed rhs in both paths (except for numerics
   in the bound path).

3) Fix ArrayContains, ArrayOverlap to avoid equality filters when there is
   an extractionFn present. Fixes a bug introduced in #14612.

* Avoid sending nonprimitives down the bound path.
gianm added a commit to gianm/druid that referenced this pull request Jul 20, 2023
@gianm
Simplify bounds/range vs selectors/equality logic in SQL planning. (a…
65eeb45 
…pache#14619)

* Simplify bounds/range vs selectors/equality logic in SQL planning.

1) Consolidate duplicate code related to Expressions#buildTimeFloorFilter.

2) Cleaner logic in Expressions#toSimpleLeafFilter: choose bounds vs range
   filter based solely on plannerContext.isUseBoundsAndSelectors, not also
   considering rhs kind. Use parsed rhs in both paths (except for numerics
   in the bound path).

3) Fix ArrayContains, ArrayOverlap to avoid equality filters when there is
   an extractionFn present. Fixes a bug introduced in apache#14612.

* Avoid sending nonprimitives down the bound path.
abhishekagarwal87 pushed a commit that referenced this pull request Jul 21, 2023
@gianm
Simplify bounds/range vs selectors/equality logic in SQL planning. (#…
5b5d495 
…14619) (#14624)

* Simplify bounds/range vs selectors/equality logic in SQL planning.

1) Consolidate duplicate code related to Expressions#buildTimeFloorFilter.

2) Cleaner logic in Expressions#toSimpleLeafFilter: choose bounds vs range
   filter based solely on plannerContext.isUseBoundsAndSelectors, not also
   considering rhs kind. Use parsed rhs in both paths (except for numerics
   in the bound path).

3) Fix ArrayContains, ArrayOverlap to avoid equality filters when there is
   an extractionFn present. Fixes a bug introduced in #14612.

* Avoid sending nonprimitives down the bound path.
sergioferragut pushed a commit to sergioferragut/druid that referenced this pull request Jul 21, 2023
@clintropolis @sergioferragut
remove extractionFn from equality, null, and range filters (apache#14612
bbf3780 
)

* remove extractionFn from equality, null, and range filters
changes:
* EqualityFilter, NullFilter, and RangeFilter no longer support extractionFn
* SQL planner will use ExpressionFilter in the small number of cases where an extractionFn would have been used if sqlUseBoundsAndSelectors is set to false instead of equality/null/range filters
* fix bugs and add tests with serde, equals, and cache key for null, equality, and range filters

* test coverage fixes bugs

* adjust

* adjust again

* so persnickety
sergioferragut pushed a commit to sergioferragut/druid that referenced this pull request Jul 21, 2023
@gianm @sergioferragut
Simplify bounds/range vs selectors/equality logic in SQL planning. (a…
79c1e62 
…pache#14619)

* Simplify bounds/range vs selectors/equality logic in SQL planning.

1) Consolidate duplicate code related to Expressions#buildTimeFloorFilter.

2) Cleaner logic in Expressions#toSimpleLeafFilter: choose bounds vs range
   filter based solely on plannerContext.isUseBoundsAndSelectors, not also
   considering rhs kind. Use parsed rhs in both paths (except for numerics
   in the bound path).

3) Fix ArrayContains, ArrayOverlap to avoid equality filters when there is
   an extractionFn present. Fixes a bug introduced in apache#14612.

* Avoid sending nonprimitives down the bound path.
teyeheimans pushed a commit to level23/druid-client that referenced this pull request Mar 19, 2024
v4.0.0
8f89806 
- Removed GroupByV1. GroupBy v1 is a legacy engine and has not been supported since 2021.
- **Removed extraction functions.** Extraction functions are deprecated in druid and
  are NOT build in the new NullFilter, EqualityFilter and RangeFilter. These are deprecated and
  should be replaced by expressions. This is poorly documented, but can be found here:
  apache/druid#14612
  Therefore, we have decided to remove the extraction functionality completely. You can replace the functionality of
  extraction functions with expressions (and virtual columns). See below for some examples.
- Added `RangeFilter` which replaces the `BoundFilter`. See NULL usage sector below.
- Added `EqualityFilter` which replaces the `SelectorFilter`. See NULL usage sector below.
- `whereBetween` now uses the `range` filter. The `whereBetween` doesn't accept an SortingOrder anymore, but a DataType.
- The `whereFlags()` method does not accept `$useJavascript` anymore, as it used an expression filter.
- Removed `orWhereNotColumn`. This was a left-over and should have been removed in v3.
- Added support for authentication in the druid client.
@teyeheimans teyeheimans mentioned this pull request Mar 19, 2024
Merged
teyeheimans added a commit to level23/druid-client that referenced this pull request Mar 19, 2024
@teyeheimans
V4.0 (#58)
d45e421 
* v4.0.0

- Removed GroupByV1. GroupBy v1 is a legacy engine and has not been supported since 2021.
- **Removed extraction functions.** Extraction functions are deprecated in druid and
  are NOT build in the new NullFilter, EqualityFilter and RangeFilter. These are deprecated and
  should be replaced by expressions. This is poorly documented, but can be found here:
  apache/druid#14612
  Therefore, we have decided to remove the extraction functionality completely. You can replace the functionality of
  extraction functions with expressions (and virtual columns). See below for some examples.
- Added `RangeFilter` which replaces the `BoundFilter`. See NULL usage sector below.
- Added `EqualityFilter` which replaces the `SelectorFilter`. See NULL usage sector below.
- `whereBetween` now uses the `range` filter. The `whereBetween` doesn't accept an SortingOrder anymore, but a DataType.
- The `whereFlags()` method does not accept `$useJavascript` anymore, as it used an expression filter.
- Removed `orWhereNotColumn`. This was a left-over and should have been removed in v3.
- Added support for authentication in the druid client.
* Added whereArrayContains
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gianm gianm gianm approved these changes

Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
27.0
Development

Successfully merging this pull request may close these issues.
2 participants
@clintropolis @gianm