diff --git a/src/io/csv.c b/src/io/csv.c index d079c4c0..832b4721 100644 --- a/src/io/csv.c +++ b/src/io/csv.c @@ -131,7 +131,13 @@ typedef enum { CSV_TYPE_DATE, CSV_TYPE_TIME, CSV_TYPE_TIMESTAMP, - CSV_TYPE_GUID + CSV_TYPE_GUID, + /* Narrow-int parse types — selected only via explicit schema (never from + * inference, so they do not appear in promote_csv_type). Each parses the + * field as int64 and narrows at write time to match the column width. */ + CSV_TYPE_U8, + CSV_TYPE_I16, + CSV_TYPE_I32 } csv_type_t; static csv_type_t detect_type(const char* f, size_t len) { @@ -795,6 +801,9 @@ static void csv_parse_fn(void* arg, uint32_t worker_id, for (; c < ctx->n_cols; c++) { switch (ctx->col_types[c]) { case CSV_TYPE_BOOL: ((uint8_t*)ctx->col_data[c])[row] = 0; break; + case CSV_TYPE_U8: ((uint8_t*)ctx->col_data[c])[row] = 0; break; + case CSV_TYPE_I16: ((int16_t*)ctx->col_data[c])[row] = 0; break; + case CSV_TYPE_I32: ((int32_t*)ctx->col_data[c])[row] = 0; break; case CSV_TYPE_I64: ((int64_t*)ctx->col_data[c])[row] = 0; break; case CSV_TYPE_F64: ((double*)ctx->col_data[c])[row] = 0.0; break; case CSV_TYPE_DATE: ((int32_t*)ctx->col_data[c])[row] = 0; break; @@ -846,6 +855,36 @@ static void csv_parse_fn(void* arg, uint32_t worker_id, } break; } + case CSV_TYPE_U8: { + bool is_null; + int64_t v = fast_i64(fld, flen, &is_null); + ((uint8_t*)ctx->col_data[c])[row] = (uint8_t)v; + if (is_null) { + ctx->col_nullmaps[c][row >> 3] |= (uint8_t)(1u << (row & 7)); + my_had_null[c] = true; + } + break; + } + case CSV_TYPE_I16: { + bool is_null; + int64_t v = fast_i64(fld, flen, &is_null); + ((int16_t*)ctx->col_data[c])[row] = (int16_t)v; + if (is_null) { + ctx->col_nullmaps[c][row >> 3] |= (uint8_t)(1u << (row & 7)); + my_had_null[c] = true; + } + break; + } + case CSV_TYPE_I32: { + bool is_null; + int64_t v = fast_i64(fld, flen, &is_null); + ((int32_t*)ctx->col_data[c])[row] = (int32_t)v; + if (is_null) { + ctx->col_nullmaps[c][row >> 3] |= (uint8_t)(1u << (row & 7)); + my_had_null[c] = true; + } + break; + } case CSV_TYPE_F64: { bool is_null; double v = fast_f64(fld, flen, &is_null); @@ -953,6 +992,9 @@ static void csv_parse_serial(const char* buf, size_t buf_size, for (; c < n_cols; c++) { switch (col_types[c]) { case CSV_TYPE_BOOL: ((uint8_t*)col_data[c])[row] = 0; break; + case CSV_TYPE_U8: ((uint8_t*)col_data[c])[row] = 0; break; + case CSV_TYPE_I16: ((int16_t*)col_data[c])[row] = 0; break; + case CSV_TYPE_I32: ((int32_t*)col_data[c])[row] = 0; break; case CSV_TYPE_I64: ((int64_t*)col_data[c])[row] = 0; break; case CSV_TYPE_F64: ((double*)col_data[c])[row] = 0.0; break; case CSV_TYPE_DATE: ((int32_t*)col_data[c])[row] = 0; break; @@ -1004,6 +1046,36 @@ static void csv_parse_serial(const char* buf, size_t buf_size, } break; } + case CSV_TYPE_U8: { + bool is_null; + int64_t v = fast_i64(fld, flen, &is_null); + ((uint8_t*)col_data[c])[row] = (uint8_t)v; + if (is_null) { + col_nullmaps[c][row >> 3] |= (uint8_t)(1u << (row & 7)); + col_had_null[c] = true; + } + break; + } + case CSV_TYPE_I16: { + bool is_null; + int64_t v = fast_i64(fld, flen, &is_null); + ((int16_t*)col_data[c])[row] = (int16_t)v; + if (is_null) { + col_nullmaps[c][row >> 3] |= (uint8_t)(1u << (row & 7)); + col_had_null[c] = true; + } + break; + } + case CSV_TYPE_I32: { + bool is_null; + int64_t v = fast_i64(fld, flen, &is_null); + ((int32_t*)col_data[c])[row] = (int32_t)v; + if (is_null) { + col_nullmaps[c][row >> 3] |= (uint8_t)(1u << (row & 7)); + col_had_null[c] = true; + } + break; + } case CSV_TYPE_F64: { bool is_null; double v = fast_f64(fld, flen, &is_null); @@ -1298,6 +1370,9 @@ ray_t* ray_read_csv_opts(const char* path, char delimiter, bool header, for (int c = 0; c < ncols; c++) { switch (resolved_types[c]) { case RAY_BOOL: parse_types[c] = CSV_TYPE_BOOL; break; + case RAY_U8: parse_types[c] = CSV_TYPE_U8; break; + case RAY_I16: parse_types[c] = CSV_TYPE_I16; break; + case RAY_I32: parse_types[c] = CSV_TYPE_I32; break; case RAY_I64: parse_types[c] = CSV_TYPE_I64; break; case RAY_F64: parse_types[c] = CSV_TYPE_F64; break; case RAY_DATE: parse_types[c] = CSV_TYPE_DATE; break; diff --git a/src/ops/sort.c b/src/ops/sort.c index a5875e27..24c95e4c 100644 --- a/src/ops/sort.c +++ b/src/ops/sort.c @@ -1020,7 +1020,28 @@ void radix_encode_fn(void* arg, uint32_t wid, int64_t start, int64_t end) { } case RAY_I16: { const int16_t* d = (const int16_t*)c->data; - if (c->desc) { + bool has_nulls = c->col && (c->col->attrs & RAY_ATTR_HAS_NULLS); + bool nf = c->nulls_first; + bool desc = c->desc; + if (has_nulls) { + /* Shift non-null encoded values up by 1 so the null + * sentinel sits outside the [1, 0x10000] data range and + * cannot tie with INT16_MIN or INT16_MAX. Costs one + * extra radix-byte pass; sort_indices_ex bumps + * key_nbytes_max accordingly. */ + uint64_t null_e = (nf ^ desc) ? 0 : 0x10001ULL; + if (desc) { + for (int64_t i = start; i < end; i++) { + if (ray_vec_is_null(c->col, i)) c->keys[i] = ~null_e; + else c->keys[i] = ~((uint64_t)((uint16_t)d[i] ^ ((uint16_t)1 << 15)) + 1); + } + } else { + for (int64_t i = start; i < end; i++) { + if (ray_vec_is_null(c->col, i)) c->keys[i] = null_e; + else c->keys[i] = (uint64_t)((uint16_t)d[i] ^ ((uint16_t)1 << 15)) + 1; + } + } + } else if (desc) { for (int64_t i = start; i < end; i++) c->keys[i] = ~((uint64_t)((uint16_t)d[i] ^ ((uint16_t)1 << 15))); } else { @@ -1031,7 +1052,26 @@ void radix_encode_fn(void* arg, uint32_t wid, int64_t start, int64_t end) { } case RAY_BOOL: case RAY_U8: { const uint8_t* d = (const uint8_t*)c->data; - if (c->desc) { + bool has_nulls = c->col && (c->col->attrs & RAY_ATTR_HAS_NULLS); + bool nf = c->nulls_first; + bool desc = c->desc; + if (has_nulls) { + /* Shift non-null encoded values up by 1; null sentinel + * lives at 0 (NF) or 0x101 (NL), beyond every U8/BOOL + * data value. */ + uint64_t null_e = (nf ^ desc) ? 0 : 0x101ULL; + if (desc) { + for (int64_t i = start; i < end; i++) { + if (ray_vec_is_null(c->col, i)) c->keys[i] = ~null_e; + else c->keys[i] = ~((uint64_t)d[i] + 1); + } + } else { + for (int64_t i = start; i < end; i++) { + if (ray_vec_is_null(c->col, i)) c->keys[i] = null_e; + else c->keys[i] = (uint64_t)d[i] + 1; + } + } + } else if (desc) { for (int64_t i = start; i < end; i++) c->keys[i] = ~(uint64_t)d[i]; } else { @@ -2466,6 +2506,15 @@ static ray_t* sort_indices_ex(ray_t** cols, uint8_t* descs, uint8_t* nulls_first if (can_radix && n_cols == 1) { /* --- Single-key sort --- */ uint8_t key_nbytes_max = radix_key_bytes(cols[0]->type); + /* Narrow-int + has_nulls uses a +1-shifted encoding so + * the null sentinel sits one byte beyond the data + * range; reserve that extra byte for the radix pass. */ + if ((cols[0]->attrs & RAY_ATTR_HAS_NULLS) && + (cols[0]->type == RAY_BOOL || cols[0]->type == RAY_U8 || + cols[0]->type == RAY_I16) && + key_nbytes_max < 8) { + key_nbytes_max++; + } /* Skip pool for small arrays - dispatch overhead dominates */ ray_pool_t* sk_pool = (nrows >= SMALL_POOL_THRESHOLD) ? pool : NULL; @@ -3365,7 +3414,11 @@ ray_t* ray_sort(ray_t** cols, uint8_t* descs, uint8_t* nulls_first, &sorted_keys, &keys_hdr); if (!idx || RAY_IS_ERR(idx)) return idx; - if (sorted_keys && !RAY_IS_SYM(cols[0]->type)) { + bool c0_shifted = (cols[0]->attrs & RAY_ATTR_HAS_NULLS) && + (cols[0]->type == RAY_BOOL || + cols[0]->type == RAY_U8 || + cols[0]->type == RAY_I16); + if (sorted_keys && !RAY_IS_SYM(cols[0]->type) && !c0_shifted) { /* Decode path: sequential writes, no random access */ ray_t* result = ray_vec_new(cols[0]->type, nrows); if (!result || RAY_IS_ERR(result)) { @@ -3581,9 +3634,16 @@ ray_t* exec_sort(ray_graph_t* g, ray_op_t* op, ray_t* tbl, int64_t limit) { /* Decode-gather optimisation: decode the sort key column directly from * sorted radix keys (sequential writes) instead of random-access gather. - * Only for single-key, non-SYM sorts where radix keys are available. */ + * Only for single-key, non-SYM sorts where radix keys are available. + * Narrow-int + has_nulls uses a +1-shifted encoding that radix_decode_into + * doesn't invert, so fall back to gather there. */ + int8_t sk0_type = sort_vecs[0] ? sort_vecs[0]->type : 0; + bool sk0_shifted = sort_vecs[0] && + (sort_vecs[0]->attrs & RAY_ATTR_HAS_NULLS) && + (sk0_type == RAY_BOOL || sk0_type == RAY_U8 || + sk0_type == RAY_I16); int64_t sort_key_sym = -1; - if (sorted_keys && n_sort == 1 && !RAY_IS_SYM(sort_vecs[0]->type)) { + if (sorted_keys && n_sort == 1 && !RAY_IS_SYM(sk0_type) && !sk0_shifted) { ray_op_ext_t* key_ext = find_ext(g, ext->sort.columns[0]->id); if (key_ext && key_ext->base.opcode == OP_SCAN) sort_key_sym = key_ext->sym; @@ -3910,9 +3970,15 @@ ray_t* sort_table_by_keys(ray_t* tbl, ray_t* keys, uint8_t descending) { /* Decode sort key column directly from sorted radix keys when * available — sequential write, much faster than random-access * gather. Only for single-key sorts where sort_indices_ex - * produced sorted_keys (non-packed path). */ + * produced sorted_keys (non-packed path). Narrow-int + has_nulls + * uses a +1-shifted encoding that radix_decode_into doesn't invert, + * so fall back to gather in that case. */ int64_t decode_col_idx = -1; - if (sorted_keys && n_keys == 1 && !RAY_IS_SYM(key_cols[0]->type)) { + int8_t k0_type = key_cols[0]->type; + bool k0_shifted = (key_cols[0]->attrs & RAY_ATTR_HAS_NULLS) && + (k0_type == RAY_BOOL || k0_type == RAY_U8 || + k0_type == RAY_I16); + if (sorted_keys && n_keys == 1 && !RAY_IS_SYM(k0_type) && !k0_shifted) { for (int64_t c = 0; c < ncols; c++) { if (col_names[c] == key_ids[0] && new_cols[c]) { decode_col_idx = c; @@ -3922,7 +3988,7 @@ ray_t* sort_table_by_keys(ray_t* tbl, ray_t* keys, uint8_t descending) { } if (decode_col_idx >= 0) { radix_decode_into(ray_data(new_cols[decode_col_idx]), - key_cols[0]->type, sorted_keys, + k0_type, sorted_keys, nrows, descs[0]); } diff --git a/src/ops/window.c b/src/ops/window.c index f462f19b..b118267f 100644 --- a/src/ops/window.c +++ b/src/ops/window.c @@ -757,6 +757,15 @@ ray_t* exec_window(ray_graph_t* g, ray_op_t* op, ray_t* tbl) { if (can_radix && n_sort == 1) { /* Single-key sort */ uint8_t key_nbytes = radix_key_bytes(sort_vecs[0]->type); + /* Narrow-int + has_nulls uses a +1-shifted encoding — + * keep the radix pass aligned with the wider key. */ + if ((sort_vecs[0]->attrs & RAY_ATTR_HAS_NULLS) && + (sort_vecs[0]->type == RAY_BOOL || + sort_vecs[0]->type == RAY_U8 || + sort_vecs[0]->type == RAY_I16) && + key_nbytes < 8) { + key_nbytes++; + } ray_pool_t* sk_pool = (nrows >= SMALL_POOL_THRESHOLD) ? pool : NULL; ray_t *keys_hdr; uint64_t* keys = (uint64_t*)scratch_alloc(&keys_hdr, diff --git a/test/test_csv.c b/test/test_csv.c index 041c64c3..e97bcdb3 100644 --- a/test/test_csv.c +++ b/test/test_csv.c @@ -1131,6 +1131,171 @@ static test_result_t test_csv_sym_narrowing(void) { PASS(); } +/* Explicit narrow-int schema: regression for the parse_types map that + * silently routed RAY_U8/RAY_I16/RAY_I32 to CSV_TYPE_STR. In that + * state csv_intern_strings would write 4-byte sym IDs into the smaller + * column buffer, overflowing the heap for U8/I16 (eventual SIGSEGV in + * later allocators) and producing sym IDs instead of integers for I32. */ + +static test_result_t test_csv_explicit_u8_schema(void) { + ray_heap_init(); + (void)ray_sym_init(); + + FILE* f = fopen(TMP_CSV, "w"); + fprintf(f, "v\n"); + /* 10 000 rows ⇒ parallel parse path; values 0..255 cycling so the + * truncated bytes fully exercise the U8 range. */ + const int N = 10000; + for (int i = 0; i < N; i++) fprintf(f, "%d\n", (i + 1) % 256); + fclose(f); + + int8_t schema[1] = { RAY_U8 }; + ray_t* loaded = ray_read_csv_opts(TMP_CSV, 0, true, schema, 1); + TEST_ASSERT_FALSE(RAY_IS_ERR(loaded)); + TEST_ASSERT_EQ_I(ray_table_nrows(loaded), N); + + ray_t* col = ray_table_get_col_idx(loaded, 0); + TEST_ASSERT_EQ_I(col->type, RAY_U8); + const uint8_t* d = (const uint8_t*)ray_data(col); + TEST_ASSERT_EQ_I((int)d[0], 1); + TEST_ASSERT_EQ_I((int)d[N - 1], (int)((N) % 256)); + /* Spot-check a mid value to make sure we didn't get sym IDs. */ + TEST_ASSERT_EQ_I((int)d[100], (int)(101 % 256)); + /* No null sentinels expected; column must not advertise nulls. */ + TEST_ASSERT_FALSE(col->attrs & RAY_ATTR_HAS_NULLS); + + /* Sort it — without the fix this segfaults inside packed_radix_sort_run + * on the corrupted heap. */ + ray_t* col_arg = col; + uint8_t desc = 0, nf = 1; + ray_t* idx = ray_sort_indices(&col_arg, &desc, &nf, 1, N); + TEST_ASSERT_FALSE(RAY_IS_ERR(idx)); + ray_release(idx); + + ray_release(loaded); + unlink(TMP_CSV); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + +static test_result_t test_csv_explicit_i16_schema_with_nulls(void) { + ray_heap_init(); + (void)ray_sym_init(); + + FILE* f = fopen(TMP_CSV, "w"); + fprintf(f, "v\n"); + const int N = 1500; + for (int i = 0; i < N; i++) { + if (i % 200 == 0) fprintf(f, "\n"); /* empty → null */ + else fprintf(f, "%d\n", (i % 17) - 8); /* range -8..8 */ + } + fclose(f); + + int8_t schema[1] = { RAY_I16 }; + ray_t* loaded = ray_read_csv_opts(TMP_CSV, 0, true, schema, 1); + TEST_ASSERT_FALSE(RAY_IS_ERR(loaded)); + TEST_ASSERT_EQ_I(ray_table_nrows(loaded), N); + + ray_t* col = ray_table_get_col_idx(loaded, 0); + TEST_ASSERT_EQ_I(col->type, RAY_I16); + TEST_ASSERT_TRUE(col->attrs & RAY_ATTR_HAS_NULLS); + + /* Every 200th row must be null, others must hold the parsed integer. */ + const int16_t* d = (const int16_t*)ray_data(col); + int null_count = 0; + for (int i = 0; i < N; i++) { + if (i % 200 == 0) { + TEST_ASSERT_TRUE(ray_vec_is_null(col, i)); + null_count++; + } else { + TEST_ASSERT_FALSE(ray_vec_is_null(col, i)); + TEST_ASSERT_EQ_I((int)d[i], (i % 17) - 8); + } + } + TEST_ASSERT_EQ_I(null_count, (N + 199) / 200); + + ray_release(loaded); + unlink(TMP_CSV); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + +static test_result_t test_csv_explicit_i32_schema(void) { + ray_heap_init(); + (void)ray_sym_init(); + + FILE* f = fopen(TMP_CSV, "w"); + fprintf(f, "v\n"); + const int N = 500; + for (int i = 0; i < N; i++) fprintf(f, "%d\n", -100000 + i * 137); + fclose(f); + + int8_t schema[1] = { RAY_I32 }; + ray_t* loaded = ray_read_csv_opts(TMP_CSV, 0, true, schema, 1); + TEST_ASSERT_FALSE(RAY_IS_ERR(loaded)); + ray_t* col = ray_table_get_col_idx(loaded, 0); + TEST_ASSERT_EQ_I(col->type, RAY_I32); + + /* Without the fix this column would hold sym IDs (1, 2, 3, …) instead + * of the actual integers. */ + const int32_t* d = (const int32_t*)ray_data(col); + for (int i = 0; i < N; i++) + TEST_ASSERT_EQ_I((int)d[i], -100000 + i * 137); + + ray_release(loaded); + unlink(TMP_CSV); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + +static test_result_t test_csv_explicit_u8_schema_serial(void) { + /* Force the serial parse path (n_rows ≤ 8192) and exercise truncated-row + * fill defaults plus null-column path. */ + ray_heap_init(); + (void)ray_sym_init(); + + FILE* f = fopen(TMP_CSV, "w"); + fprintf(f, "a,b\n"); + /* 200 rows; second column missing on every 50th row → triggers + * past-row-boundary fill in the parser. */ + for (int i = 0; i < 200; i++) { + if (i % 50 == 0) fprintf(f, "%d\n", i); + else fprintf(f, "%d,%d\n", i, i + 1); + } + fclose(f); + + int8_t schema[2] = { RAY_U8, RAY_U8 }; + ray_t* loaded = ray_read_csv_opts(TMP_CSV, 0, true, schema, 2); + TEST_ASSERT_FALSE(RAY_IS_ERR(loaded)); + TEST_ASSERT_EQ_I(ray_table_nrows(loaded), 200); + + ray_t* a = ray_table_get_col_idx(loaded, 0); + ray_t* b = ray_table_get_col_idx(loaded, 1); + TEST_ASSERT_EQ_I(a->type, RAY_U8); + TEST_ASSERT_EQ_I(b->type, RAY_U8); + TEST_ASSERT_TRUE(b->attrs & RAY_ATTR_HAS_NULLS); + + const uint8_t* ad = (const uint8_t*)ray_data(a); + const uint8_t* bd = (const uint8_t*)ray_data(b); + for (int i = 0; i < 200; i++) { + TEST_ASSERT_EQ_I((int)ad[i], i % 256); + if (i % 50 == 0) TEST_ASSERT_TRUE(ray_vec_is_null(b, i)); + else { + TEST_ASSERT_FALSE(ray_vec_is_null(b, i)); + TEST_ASSERT_EQ_I((int)bd[i], (i + 1) % 256); + } + } + + ray_release(loaded); + unlink(TMP_CSV); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + const test_entry_t csv_entries[] = { { "csv/roundtrip_i64", test_csv_roundtrip_i64, NULL, NULL }, { "csv/roundtrip_guid", test_csv_guid_roundtrip, NULL, NULL }, @@ -1167,5 +1332,13 @@ const test_entry_t csv_entries[] = { { "csv/header_needs_quoting", test_csv_header_needs_quoting, NULL, NULL }, { "csv/parallel_parse", test_csv_parallel_parse, NULL, NULL }, { "csv/sym_narrowing", test_csv_sym_narrowing, NULL, NULL }, + /* Narrow-int explicit schema (regression for missing parse_types map + * entries that routed U8/I16/I32 to STR and corrupted the heap). */ + { "csv/explicit_u8_schema", test_csv_explicit_u8_schema, NULL, NULL }, + { "csv/explicit_i16_schema_with_nulls", + test_csv_explicit_i16_schema_with_nulls, NULL, NULL }, + { "csv/explicit_i32_schema", test_csv_explicit_i32_schema, NULL, NULL }, + { "csv/explicit_u8_schema_serial", + test_csv_explicit_u8_schema_serial, NULL, NULL }, { NULL, NULL, NULL, NULL }, }; diff --git a/test/test_sort.c b/test/test_sort.c index 354d8afa..67296717 100644 --- a/test/test_sort.c +++ b/test/test_sort.c @@ -865,6 +865,231 @@ static test_result_t test_sort_multi_col(void) { PASS(); } +/* ══════════════════════════════════════════════════════════════════ + * Narrow-int single-key sort with nulls — regression for + * radix_encode_fn (I16, BOOL, U8) which previously did not consult + * the column's null bitmap and aliased nulls onto whatever the + * underlying byte data happened to be. Each test forces nrows above + * the comparison-merge threshold (64) so we exercise the radix path, + * and seeds non-null values around the encoded null sentinel so the + * underlying byte data cannot mask the bug. + * ══════════════════════════════════════════════════════════════════ */ + +/* Build an I16 vec with `data` and mark `null_pos` rows null. */ +static ray_t* build_i16_with_nulls(const int16_t* data, int64_t n, + const int64_t* null_pos, int64_t n_nulls) { + ray_t* vec = ray_vec_from_raw(RAY_I16, data, n); + for (int64_t i = 0; i < n_nulls; i++) + ray_vec_set_null(vec, null_pos[i], true); + return vec; +} + +static test_result_t test_sort_i16_nulls_first_with_negatives(void) { + ray_heap_init(); + ray_sym_init(); + + /* 96 rows: span includes negatives so the encoded null sentinel + * (0 ASC) cannot land "by accident" between negatives and positives. + * Without the fix, nulls land where the underlying data byte (here + * zeroed by the helper) happens to encode, i.e. between -1 and 0. */ + enum { N = 96 }; + int16_t data[N]; + for (int i = 0; i < N; i++) data[i] = (int16_t)(i - 48); /* -48..47 */ + int64_t null_pos[] = {3, 17, 50, 80}; + ray_t* vec = build_i16_with_nulls(data, N, null_pos, 4); + + uint8_t desc = 0, nf = 1; + ray_t* idx = ray_sort_indices(&vec, &desc, &nf, 1, N); + TEST_ASSERT_FALSE(RAY_IS_ERR(idx)); + const int64_t* idxd = (const int64_t*)ray_data(idx); + + /* First 4 must be null rows */ + for (int i = 0; i < 4; i++) + TEST_ASSERT_FMT(ray_vec_is_null(vec, idxd[i]), + "i16 nulls-first: position %d is not null", i); + /* Remaining must be non-decreasing and non-null */ + for (int64_t i = 5; i < N; i++) { + TEST_ASSERT_FALSE(ray_vec_is_null(vec, idxd[i])); + TEST_ASSERT_FMT(data[idxd[i]] >= data[idxd[i-1]], + "i16 asc: out of order at %lld", (long long)i); + } + + ray_release(idx); + ray_release(vec); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + +static test_result_t test_sort_i16_nulls_last_desc(void) { + ray_heap_init(); + ray_sym_init(); + + enum { N = 96 }; + int16_t data[N]; + for (int i = 0; i < N; i++) data[i] = (int16_t)(i - 48); + int64_t null_pos[] = {1, 9, 60, 95}; + ray_t* vec = build_i16_with_nulls(data, N, null_pos, 4); + + uint8_t desc = 1, nf = 0; /* DESC, nulls LAST */ + ray_t* idx = ray_sort_indices(&vec, &desc, &nf, 1, N); + TEST_ASSERT_FALSE(RAY_IS_ERR(idx)); + const int64_t* idxd = (const int64_t*)ray_data(idx); + + /* Last 4 must be null rows */ + for (int i = 0; i < 4; i++) + TEST_ASSERT_FMT(ray_vec_is_null(vec, idxd[N - 1 - i]), + "i16 nulls-last: position %d from end is not null", i); + /* Leading non-null prefix must be non-increasing */ + for (int64_t i = 1; i < N - 4; i++) { + TEST_ASSERT_FALSE(ray_vec_is_null(vec, idxd[i])); + TEST_ASSERT_FMT(data[idxd[i]] <= data[idxd[i-1]], + "i16 desc: out of order at %lld", (long long)i); + } + + ray_release(idx); + ray_release(vec); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + +static test_result_t test_sort_i16_nulls_first_desc(void) { + /* DESC × nulls-first: encoded null sentinel UINT64_MAX, must beat + * even INT16_MIN at the leading edge. */ + ray_heap_init(); + ray_sym_init(); + + enum { N = 96 }; + int16_t data[N]; + for (int i = 0; i < N; i++) data[i] = (int16_t)(i - 48); + int64_t null_pos[] = {0, 47, 70}; + ray_t* vec = build_i16_with_nulls(data, N, null_pos, 3); + + uint8_t desc = 1, nf = 1; + ray_t* idx = ray_sort_indices(&vec, &desc, &nf, 1, N); + TEST_ASSERT_FALSE(RAY_IS_ERR(idx)); + const int64_t* idxd = (const int64_t*)ray_data(idx); + + for (int i = 0; i < 3; i++) + TEST_ASSERT_TRUE(ray_vec_is_null(vec, idxd[i])); + + ray_release(idx); + ray_release(vec); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + +static test_result_t test_sort_u8_nulls_last_asc(void) { + /* U8 ASC × nulls-last: with the bug, nulls follow the underlying + * byte data (zeroed) and would group with the smallest values + * instead of trailing the result. */ + ray_heap_init(); + ray_sym_init(); + + enum { N = 100 }; + uint8_t data[N]; + for (int i = 0; i < N; i++) data[i] = (uint8_t)(i + 1); /* 1..100, no zeros */ + ray_t* vec = ray_vec_from_raw(RAY_U8, data, N); + int64_t null_pos[] = {2, 33, 77}; + for (int i = 0; i < 3; i++) ray_vec_set_null(vec, null_pos[i], true); + + uint8_t desc = 0, nf = 0; /* ASC, nulls LAST */ + ray_t* idx = ray_sort_indices(&vec, &desc, &nf, 1, N); + TEST_ASSERT_FALSE(RAY_IS_ERR(idx)); + const int64_t* idxd = (const int64_t*)ray_data(idx); + + /* Last three must be nulls */ + for (int i = 0; i < 3; i++) + TEST_ASSERT_FMT(ray_vec_is_null(vec, idxd[N - 1 - i]), + "u8 nulls-last: pos %d from end is not null", i); + /* Leading prefix non-decreasing */ + for (int64_t i = 1; i < N - 3; i++) { + TEST_ASSERT_FALSE(ray_vec_is_null(vec, idxd[i])); + TEST_ASSERT_TRUE(data[idxd[i]] >= data[idxd[i-1]]); + } + + ray_release(idx); + ray_release(vec); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + +static test_result_t test_sort_u8_nulls_first_desc(void) { + /* DESC × nulls-first: encoded null = ~0 = UINT64_MAX, sorts before + * even 0xFF. Underlying data here intentionally contains 0xFF so + * the bug's natural-byte-order behavior cannot mimic the fix. */ + ray_heap_init(); + ray_sym_init(); + + enum { N = 100 }; + uint8_t data[N]; + for (int i = 0; i < N; i++) data[i] = (uint8_t)(150 + i % 50); /* 150..199 */ + ray_t* vec = ray_vec_from_raw(RAY_U8, data, N); + int64_t null_pos[] = {10, 50, 90}; + for (int i = 0; i < 3; i++) ray_vec_set_null(vec, null_pos[i], true); + + uint8_t desc = 1, nf = 1; + ray_t* idx = ray_sort_indices(&vec, &desc, &nf, 1, N); + TEST_ASSERT_FALSE(RAY_IS_ERR(idx)); + const int64_t* idxd = (const int64_t*)ray_data(idx); + + for (int i = 0; i < 3; i++) + TEST_ASSERT_TRUE(ray_vec_is_null(vec, idxd[i])); + /* Tail non-increasing */ + for (int64_t i = 4; i < N; i++) { + TEST_ASSERT_FALSE(ray_vec_is_null(vec, idxd[i])); + TEST_ASSERT_TRUE(data[idxd[i]] <= data[idxd[i-1]]); + } + + ray_release(idx); + ray_release(vec); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + +static test_result_t test_sort_bool_nulls_first(void) { + /* BOOL shares the U8 encode path; nulls must still respect the + * requested boundary and not get folded into the false bucket. */ + ray_heap_init(); + ray_sym_init(); + + enum { N = 100 }; + uint8_t data[N]; + for (int i = 0; i < N; i++) data[i] = (uint8_t)(i & 1); + ray_t* vec = ray_vec_from_raw(RAY_BOOL, data, N); + int64_t null_pos[] = {0, 25, 50, 99}; + for (int i = 0; i < 4; i++) ray_vec_set_null(vec, null_pos[i], true); + + uint8_t desc = 0, nf = 1; + ray_t* idx = ray_sort_indices(&vec, &desc, &nf, 1, N); + TEST_ASSERT_FALSE(RAY_IS_ERR(idx)); + const int64_t* idxd = (const int64_t*)ray_data(idx); + + for (int i = 0; i < 4; i++) + TEST_ASSERT_FMT(ray_vec_is_null(vec, idxd[i]), + "bool nulls-first: pos %d is not null", i); + + /* Among non-nulls, all 0s come before all 1s. */ + int saw_one = 0; + for (int64_t i = 4; i < N; i++) { + TEST_ASSERT_FALSE(ray_vec_is_null(vec, idxd[i])); + if (data[idxd[i]] == 1) saw_one = 1; + else TEST_ASSERT_FMT(saw_one == 0, + "bool asc: a 0 appears after a 1 at %lld", + (long long)i); + } + + ray_release(idx); + ray_release(vec); + ray_sym_destroy(); + ray_heap_destroy(); + PASS(); +} + /* ─── Entry table ────────────────────────────────────────────────── */ const test_entry_t sort_entries[] = { @@ -901,5 +1126,13 @@ const test_entry_t sort_entries[] = { { "sort/nulls_first", test_sort_nulls_first, NULL, NULL }, /* ray_sort multi-column path */ { "sort/multi_col", test_sort_multi_col, NULL, NULL }, + /* Narrow-int single-key null encoding (regression for radix_encode_fn + * I16/BOOL/U8 cases that ignored the null bitmap). */ + { "sort/i16_nulls_first_negs", test_sort_i16_nulls_first_with_negatives, NULL, NULL }, + { "sort/i16_nulls_last_desc", test_sort_i16_nulls_last_desc, NULL, NULL }, + { "sort/i16_nulls_first_desc", test_sort_i16_nulls_first_desc, NULL, NULL }, + { "sort/u8_nulls_last_asc", test_sort_u8_nulls_last_asc, NULL, NULL }, + { "sort/u8_nulls_first_desc", test_sort_u8_nulls_first_desc, NULL, NULL }, + { "sort/bool_nulls_first", test_sort_bool_nulls_first, NULL, NULL }, { NULL, NULL, NULL, NULL }, };