Skip to content

Fix chunk skipping range check bug #7318

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

Merged
merged 1 commit into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .unreleased/pr_7318
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes: #7318: Fix chunk skipping range filtering
83 changes: 4 additions & 79 deletions src/hypertable_restrict_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,81 +175,6 @@ dimension_restrict_info_open_add(DimensionRestrictInfoOpen *dri, StrategyNumber
return restriction_added;
}

/*
* for DIMENSION_TYPE_STATS entries, we need to use regular bounded strategies.
* We can have multiple entries satisfying the inputs. It basically becomes a
* problem of:
*
* "given an input value, find all ranges which encompass that value"
*
* For "column >= constant" (e.g id >= 9), the check has to be:
*
* start_range >= 9 OR end_range >= 9 (second check covers +INF)
*
* For "column <= constant" (e.g id <= 90), the check has to be:
*
* start_range <= 90 OR end_range <= 90 (first check covers -INF)
*
* For "column == constant" (e.g id = 9)
*
* start_range <= 9 OR end_range >= 9 (covers +INF to -INF)
*/
static bool
dimension_restrict_info_range_add(DimensionRestrictInfoOpen *dri, StrategyNumber strategy,
Oid collation, DimensionValues *dimvalues)
{
ListCell *item;
bool restriction_added = false;

/* can't handle IN/ANY with multiple values */
if (dimvalues->use_or && list_length(dimvalues->values) > 1)
return false;

foreach (item, dimvalues->values)
{
Oid restype;
Datum datum = ts_dimension_transform_value(dri->base.dimension,
collation,
PointerGetDatum(lfirst(item)),
dimvalues->type,
&restype);
int64 value = ts_time_value_to_internal_or_infinite(datum, restype);

switch (strategy)
{
case BTLessEqualStrategyNumber:
case BTLessStrategyNumber: /* e.g: id <= 90 */
if (dri->upper_strategy == InvalidStrategy || value < dri->upper_bound)
{
dri->upper_strategy = strategy;
dri->upper_bound = value;
restriction_added = true;
}
break;
case BTGreaterEqualStrategyNumber:
case BTGreaterStrategyNumber: /* e.g: id >= 9 */
if (dri->lower_strategy == InvalidStrategy || value > dri->lower_bound)
{
dri->lower_strategy = strategy;
dri->lower_bound = value;
restriction_added = true;
}
break;
case BTEqualStrategyNumber: /* e.g: id = 9 */
dri->lower_bound = value;
dri->upper_bound = value;
dri->lower_strategy = BTEqualStrategyNumber;
dri->upper_strategy = BTEqualStrategyNumber;
restriction_added = true;
break;
default:
/* unsupported strategy */
break;
}
}
return restriction_added;
}

static List *
dimension_restrict_info_get_partitions(DimensionRestrictInfoClosed *dri, Oid collation,
List *values)
Expand Down Expand Up @@ -330,10 +255,10 @@ dimension_restrict_info_add(DimensionRestrictInfo *dri, int strategy, Oid collat
values);
case DIMENSION_TYPE_STATS:
/* we reuse the DimensionRestrictInfoOpen structure for these */
return dimension_restrict_info_range_add((DimensionRestrictInfoOpen *) dri,
strategy,
collation,
values);
return dimension_restrict_info_open_add((DimensionRestrictInfoOpen *) dri,
strategy,
collation,
values);
case DIMENSION_TYPE_CLOSED:
return dimension_restrict_info_closed_add((DimensionRestrictInfoClosed *) dri,
strategy,
Expand Down
89 changes: 8 additions & 81 deletions src/ts_catalog/chunk_column_stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -1071,7 +1071,6 @@ ts_chunk_column_stats_get_chunk_ids_by_scan(DimensionRestrictInfo *dri)
ScanIterator it;
List *chunkids = NIL;
DimensionRestrictInfoOpen *open;
bool check_both = false;

Assert(dri && dri->dimension->type == DIMENSION_TYPE_STATS);

Expand All @@ -1082,25 +1081,6 @@ ts_chunk_column_stats_get_chunk_ids_by_scan(DimensionRestrictInfo *dri)

open = (DimensionRestrictInfoOpen *) dri;

/*
* Since we need to find chunks with overlapping ranges. We use checks on both the
* lower_bound and the upper_bound.
*/
if (open->upper_strategy != InvalidStrategy && open->lower_strategy != InvalidStrategy)
check_both = true;

if (open->upper_strategy == InvalidStrategy)
{
open->upper_strategy = open->lower_strategy;
open->upper_bound = open->lower_bound;
}

if (open->lower_strategy == InvalidStrategy)
{
open->lower_strategy = open->upper_strategy;
open->lower_bound = open->upper_bound;
}

/*
* We need to get all chunks matching the hypertable ID and the column name.
*/
Expand Down Expand Up @@ -1143,18 +1123,6 @@ ts_chunk_column_stats_get_chunk_ids_by_scan(DimensionRestrictInfo *dri)
goto done;
}

/*
* If both upper and lower bounds have been specified then we need to check
* if that range overlaps (subset, superset, intersect) with the current fd entry
* values
*/
if (check_both)
{
/* range is before or after our fd range_start/range_end values */
if (open->upper_bound < fd->range_start || open->lower_bound > fd->range_end)
goto done;
}

/*
* All data is in int8 format so we do regular comparisons. Also, it's an OR
* check so prepare to short circuit if one evaluates to true.
Expand All @@ -1166,78 +1134,37 @@ ts_chunk_column_stats_get_chunk_ids_by_scan(DimensionRestrictInfo *dri)
{
case BTLessEqualStrategyNumber: /* e.g: id <= 90 */
{
/* range_end is exclusive, so check accordingly */
if ((fd->range_end - 1) <= open->upper_bound)
matched = check_both ? false : true;
matched = fd->range_start <= open->upper_bound;
}
break;
case BTLessStrategyNumber: /* e.g: id < 90 */
{
/* range_end is exclusive, so check accordingly */
if ((fd->range_end - 1) < open->upper_bound)
matched = check_both ? false : true;
}
break;
case BTGreaterEqualStrategyNumber: /* e.g: id >= 90 */
{
/* range_end is exclusive, so check accordingly */
if ((fd->range_end - 1) >= open->upper_bound)
matched = check_both ? false : true;
}
break;
case BTGreaterStrategyNumber: /* e.g: id > 9 */
{
/* range_end is exclusive, so check accordingly */
if ((fd->range_end - 1) > open->upper_bound)
matched = check_both ? false : true;
}
break;
case BTEqualStrategyNumber: /* e.g: id == 9 */
{
/* need to check for both range_start and range_end */
if (fd->range_start <= open->lower_bound &&
(fd->range_end - 1) >= open->upper_bound)
matched = true;
matched = fd->range_start < open->upper_bound;
}
break;
default:
/* unsupported strategy */
open->upper_strategy = InvalidStrategy;
break;
}

if (matched)
if (open->upper_strategy != InvalidStrategy && !matched)
goto done;

/* range_end checks didn't match, check for range_start now */
switch (open->lower_strategy)
{
case BTLessEqualStrategyNumber:
{
if (fd->range_start <= open->lower_bound)
matched = true;
}
break;
case BTLessStrategyNumber:
{
if (fd->range_start < open->lower_bound)
matched = true;
}
break;
case BTGreaterEqualStrategyNumber:
{
if (fd->range_start >= open->lower_bound)
matched = true;
/* range_end is exclusive */
matched = (fd->range_end - 1) >= open->lower_bound;
}
break;
case BTGreaterStrategyNumber:
{
/* range_start is inclusive */
if (fd->range_start >= open->lower_bound)
matched = true;
/* range_end is exclusive */
matched = (fd->range_end - 1) > open->lower_bound;
}
break;
case BTEqualStrategyNumber:
/* already handled above with upper_strategy */
default:
/* unsupported strategy */
break;
Expand Down
Loading
Loading