Skip to content
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
24 changes: 13 additions & 11 deletions tsl/src/compression/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -715,18 +715,21 @@ decompress_chunk_impl(Chunk *uncompressed_chunk, bool if_compressed)
}

bool
is_chunk_orderby_nonnullable(CompressionSettings *settings)
is_chunk_orderby_nullhandling(CompressionSettings *settings)
{
int num_orderby = ts_array_length(settings->fd.orderby);
const char *attname;
int attnum;
for (int i = 1; i <= num_orderby; i++)
{
attname = ts_array_get_element_text(settings->fd.orderby, i);
attnum = get_attnum(settings->fd.relid, attname);
if (!AttributeNumberIsValid(attnum) || !ts_get_attnotnull(settings->fd.relid, attnum))
if (orderby_sparse_kind(settings, i) != ORDERBY_SPARSE_FIRSTLAST)
{
return false;
attname = ts_array_get_element_text(settings->fd.orderby, i);
attnum = get_attnum(settings->fd.relid, attname);
if (!AttributeNumberIsValid(attnum) || !ts_get_attnotnull(settings->fd.relid, attnum))
{
return false;
}
}
}
return true;
Expand Down Expand Up @@ -777,19 +780,18 @@ recompress_chunk_impl(Chunk *chunk, bool recompress)

/* #9444: do not recompress when order by columns are nullable, do segmentwise
* decompress/compress instead. It is due to compression min/max metadata not handling
* NULLs. When we implement chunks with min/max NULL-handling metadata, this restriction can
* be lifted.
* NULLs. This restriction is lifted with first/last metadata index.
*/
bool nullable_orderby = !is_chunk_orderby_nonnullable(chunk_settings);
if (nullable_orderby)
bool orderby_not_handling_nulls = !is_chunk_orderby_nullhandling(chunk_settings);
if (orderby_not_handling_nulls)
{
elog(ts_guc_debug_compression_path_info ? INFO : DEBUG1,
"in-memory recompression is disabled due to nullable order by, "
"in-memory recompression is disabled due to nullable order by with no firstlast, "
"performing segmentwise decompress/compress on chunk \"%s.%s\"",
NameStr(chunk->fd.schema_name),
NameStr(chunk->fd.table_name));
}
recompress_chunk_segmentwise_impl(chunk, nullable_orderby);
recompress_chunk_segmentwise_impl(chunk, orderby_not_handling_nulls);
recompressed = true;
}
else
Expand Down
2 changes: 1 addition & 1 deletion tsl/src/compression/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ extern void compression_chunk_size_catalog_insert(int32 src_chunk_id, const Rela
int64 rowcnt_frozen);
extern Datum tsl_estimate_compressed_batch_size(PG_FUNCTION_ARGS);

extern bool is_chunk_orderby_nonnullable(CompressionSettings *settings);
extern bool is_chunk_orderby_nullhandling(CompressionSettings *settings);
2 changes: 1 addition & 1 deletion tsl/src/compression/compression_dml.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ typedef struct tuple_filtering_constraints
bool vectorized_filtering;
} tuple_filtering_constraints;

bool slot_key_test(TupleTableSlot *slot, ScanKey skey);
bool slot_key_test(TupleTableSlot *slot, ScanKey skey, bool nulls_first);

ScanKeyData *build_mem_scankeys_from_slot(Oid ht_relid, CompressionSettings *settings,
Relation out_rel,
Expand Down
22 changes: 20 additions & 2 deletions tsl/src/compression/compression_scankey.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,15 @@ static bool create_segment_filter_scankey(Relation in_rel, char *segment_filter_
*
* Unlike HeapKeyTest, this function takes into account SK_ISNULL
* and works correctly when looking for null values.
*
* If slot attribute is NULL and key is NOT NULL,
* (key >= NULL) returns True for nulls_first
* and (key <= NULL) returns True for !nulls_first (i.e. for NULLS LAST).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what is (key <= NULL)?

@natalya-aksman natalya-aksman Jun 8, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

what is (key <= NULL)?

not-NULL key value sorts before NULL, the comment can be changed to be more accurate.

*/
bool
slot_key_test(TupleTableSlot *compressed_slot, ScanKey key)
slot_key_test(TupleTableSlot *compressed_slot, ScanKey key, bool nulls_first)
{
/* No need to get the datum if we are only checking for NULLs */
/* No need to get the datum if we are only checking for NULL key */
if (key->sk_flags & SK_ISNULL)
{
return slot_attisnull(compressed_slot, key->sk_attno);
Expand All @@ -43,6 +47,20 @@ slot_key_test(TupleTableSlot *compressed_slot, ScanKey key)

if (is_null)
{
/* NULL < key i.e. NULL sorts before key argument */
if (nulls_first && (key->sk_strategy == BTLessStrategyNumber ||
key->sk_strategy == BTLessEqualStrategyNumber))
{
return true;
}

/* NULL > key i.e. NULL sorts after key argument */
if (!nulls_first && (key->sk_strategy == BTGreaterStrategyNumber ||

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now that you have this check here, is handle_null_scan still needed separately?

@natalya-aksman natalya-aksman Jun 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, because the change in slot_key_test is about matching a boundary to the scan key, this method returns True if scan key fits the boundary and False otherwise, for example it is True for boundary = 5 and scan key (6, >=), i.e. (6 >= 5) = True.

We can now have Tuple_match result when key is not NULL but the boundary is NULL, i.e. for boundary = NULL, nullfirst = True and scan key (6, >=) we used to return False (no match) and now we return True (match as 6 sorts after NULL).

But we still need to handle cases of key = NULL or boundary = NULL when slot_key_test returns False. We need to check whether this tuple should go before or after a region defined by those boundaries, for example 6 goes before [10, NULL] for NULLS LAST scenario. So no change in logic for handle_null_scan, we just now return True from slot_key_test in more scenarios.

key->sk_strategy == BTGreaterEqualStrategyNumber))
{
return true;
}

return false;
}

Expand Down
14 changes: 7 additions & 7 deletions tsl/src/compression/recompress.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,16 @@ tsl_recompress_chunk_segmentwise(PG_FUNCTION_ARGS)
"compression with no "
"order by")));
}
bool nullable_orderby = !is_chunk_orderby_nonnullable(settings);
if (nullable_orderby)
bool orderby_not_handling_nulls = !is_chunk_orderby_nullhandling(settings);
if (orderby_not_handling_nulls)
{
elog(ts_guc_debug_compression_path_info ? INFO : DEBUG1,
"in-memory recompression is disabled due to nullable order by, "
"in-memory recompression is disabled due to nullable order by with no firstlast, "
"performing segmentwise decompress/compress on chunk \"%s.%s\"",
NameStr(chunk->fd.schema_name),
NameStr(chunk->fd.table_name));
}
recompress_chunk_segmentwise_impl(chunk, nullable_orderby);
recompress_chunk_segmentwise_impl(chunk, orderby_not_handling_nulls);
}

PG_RETURN_OID(uncompressed_relid);
Expand Down Expand Up @@ -1063,6 +1063,7 @@ update_orderby_scankeys(Datum *values, bool *isnulls, int num_segmentby, int num
static enum Batch_match_result
handle_null_scan(int key_flags, bool nulls_first, enum Batch_match_result result)
{
/* uncompressed tuple key is NULL */
if (key_flags & SK_ISNULL)
{
return nulls_first ? Tuple_before : Tuple_after;
Expand All @@ -1086,18 +1087,17 @@ match_tuple_batch(TupleTableSlot *compressed_slot, int num_orderby, ScanKey orde
if (num_orderby >= 1)
{
ScanKey key = &orderby_scankeys[0];
if (!slot_key_test(compressed_slot, key))
if (!slot_key_test(compressed_slot, key, nulls_first[0]))
{
return handle_null_scan(key->sk_flags, nulls_first[0], Tuple_before);
}

key = &orderby_scankeys[1];
if (!slot_key_test(compressed_slot, key))
if (!slot_key_test(compressed_slot, key, nulls_first[0]))
{
return handle_null_scan(key->sk_flags, nulls_first[0], Tuple_after);
}
}

return Tuple_match;
}

Expand Down
Loading
Loading