Skip to content

Commit d4abf2d

Browse files
singaraionasingaraiona
committed
fix(core): fixed issues found with audit
1 parent 3d72500 commit d4abf2d

6 files changed

Lines changed: 1566 additions & 274 deletions

File tree

‎core/aggr.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2873,6 +2873,7 @@ obj_p aggr_dev(obj_p val, obj_p index) {
28732873

28742874
n = index_group_count(index);
28752875

2876+
28762877
switch (val->type) {
28772878
case TYPE_I16:
28782879
case TYPE_I32:

‎core/index.c‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2478,6 +2478,31 @@ obj_p index_group_list_perfect(obj_p obj, obj_p filter) {
24782478
res = index_group_i64_scoped(ht, NULL_OBJ, scope);
24792479
drop_obj(ht);
24802480

2481+
// If there's a filter, the index maps dense positions [0..filter->len) to groups.
2482+
// Inject the filter so aggregation reads val[filter[i]] instead of val[i].
2483+
if (!is_null(filter) && !is_null(res)) {
2484+
if (index_group_type(res) == INDEX_TYPE_SHIFT) {
2485+
// SHIFT uses source[x] which is densely indexed — incompatible with filter.
2486+
// Convert to IDS: build explicit group_ids array.
2487+
i64_t *src = AS_I64(AS_LIST(res)[4]);
2488+
i64_t *keys = AS_I64(AS_LIST(res)[2]);
2489+
i64_t shift_val = AS_LIST(res)[3]->i64;
2490+
i64_t flen = filter->len;
2491+
i64_t gc = index_group_count(res);
2492+
obj_p meta = clone_obj(AS_LIST(res)[6]);
2493+
obj_p ids = I64(flen);
2494+
i64_t *ids_data = AS_I64(ids);
2495+
for (i = 0; i < flen; i++)
2496+
ids_data[i] = keys[src[i] - shift_val];
2497+
drop_obj(res);
2498+
res = index_group_build(INDEX_TYPE_IDS, gc, ids, i64(NULL_I64), NULL_OBJ, clone_obj(filter), meta);
2499+
} else {
2500+
// IDS type: just inject the filter
2501+
drop_obj(AS_LIST(res)[5]);
2502+
AS_LIST(res)[5] = clone_obj(filter);
2503+
}
2504+
}
2505+
24812506
return res;
24822507
}
24832508

‎core/math.c‎

Lines changed: 48 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2622,6 +2622,30 @@ obj_p ray_avg(obj_p x) {
26222622

26232623
// TODO: Refactoring with out sort and with parallel execution
26242624
obj_p ray_med(obj_p x) {
2625+
// Handle MAPGROUP/MAPFILTER before ray_cnt, since ops_count(MAPGROUP)
2626+
// returns the index_type enum, not the actual row count
2627+
switch (x->type) {
2628+
case TYPE_MAPGROUP:
2629+
return aggr_med(AS_LIST(x)[0], AS_LIST(x)[1]);
2630+
case TYPE_MAPFILTER: {
2631+
obj_p val = AS_LIST(x)[0];
2632+
obj_p filter = AS_LIST(x)[1];
2633+
if (val->type >= TYPE_PARTEDLIST && val->type <= TYPE_PARTEDGUID && filter->type == TYPE_PARTEDI64) {
2634+
obj_p index = vn_list(7, i64(INDEX_TYPE_PARTEDCOMMON), i64(1), NULL_OBJ, i64(NULL_I64), NULL_OBJ,
2635+
clone_obj(filter), NULL_OBJ);
2636+
obj_p res = aggr_med(val, index);
2637+
drop_obj(index);
2638+
return res;
2639+
}
2640+
obj_p collected = filter_collect(val, filter);
2641+
obj_p res = ray_med(collected);
2642+
drop_obj(collected);
2643+
return res;
2644+
}
2645+
default:
2646+
break;
2647+
}
2648+
26252649
i64_t l = ray_cnt(x)->i64;
26262650
if (l == 0)
26272651
return f64(NULL_F64);
@@ -2685,23 +2709,6 @@ obj_p ray_med(obj_p x) {
26852709

26862710
// return f64(med);
26872711

2688-
case TYPE_MAPGROUP:
2689-
return aggr_med(AS_LIST(x)[0], AS_LIST(x)[1]);
2690-
case TYPE_MAPFILTER: {
2691-
obj_p val = AS_LIST(x)[0];
2692-
obj_p filter = AS_LIST(x)[1];
2693-
if (val->type >= TYPE_PARTEDLIST && val->type <= TYPE_PARTEDGUID && filter->type == TYPE_PARTEDI64) {
2694-
obj_p index = vn_list(7, i64(INDEX_TYPE_PARTEDCOMMON), i64(1), NULL_OBJ, i64(NULL_I64), NULL_OBJ,
2695-
clone_obj(filter), NULL_OBJ);
2696-
obj_p res = aggr_med(val, index);
2697-
drop_obj(index);
2698-
return res;
2699-
}
2700-
obj_p collected = filter_collect(val, filter);
2701-
obj_p res = ray_med(collected);
2702-
drop_obj(collected);
2703-
return res;
2704-
}
27052712
case TYPE_PARTEDI16:
27062713
case TYPE_PARTEDI32:
27072714
case TYPE_PARTEDI64:
@@ -2721,6 +2728,30 @@ obj_p ray_med(obj_p x) {
27212728
}
27222729

27232730
obj_p ray_dev(obj_p x) {
2731+
// Handle MAPGROUP/MAPFILTER before ray_cnt, since ops_count(MAPGROUP)
2732+
// returns the index_type enum, not the actual row count
2733+
switch (x->type) {
2734+
case TYPE_MAPGROUP:
2735+
return aggr_dev(AS_LIST(x)[0], AS_LIST(x)[1]);
2736+
case TYPE_MAPFILTER: {
2737+
obj_p val = AS_LIST(x)[0];
2738+
obj_p filter = AS_LIST(x)[1];
2739+
if (val->type >= TYPE_PARTEDLIST && val->type <= TYPE_PARTEDGUID && filter->type == TYPE_PARTEDI64) {
2740+
obj_p index = vn_list(7, i64(INDEX_TYPE_PARTEDCOMMON), i64(1), NULL_OBJ, i64(NULL_I64), NULL_OBJ,
2741+
clone_obj(filter), NULL_OBJ);
2742+
obj_p res = aggr_dev(val, index);
2743+
drop_obj(index);
2744+
return res;
2745+
}
2746+
obj_p collected = filter_collect(val, filter);
2747+
obj_p res = ray_dev(collected);
2748+
drop_obj(collected);
2749+
return res;
2750+
}
2751+
default:
2752+
break;
2753+
}
2754+
27242755
obj_p cnt_obj = ray_cnt(x);
27252756
i64_t l = cnt_obj->i64;
27262757
drop_obj(cnt_obj);
@@ -2757,23 +2788,6 @@ obj_p ray_dev(obj_p x) {
27572788
favg = (sum_obj->f64) / (f64_t)l;
27582789
drop_obj(sum_obj);
27592790
break;
2760-
case TYPE_MAPGROUP:
2761-
return aggr_dev(AS_LIST(x)[0], AS_LIST(x)[1]);
2762-
case TYPE_MAPFILTER: {
2763-
obj_p val = AS_LIST(x)[0];
2764-
obj_p filter = AS_LIST(x)[1];
2765-
if (val->type >= TYPE_PARTEDLIST && val->type <= TYPE_PARTEDGUID && filter->type == TYPE_PARTEDI64) {
2766-
obj_p index = vn_list(7, i64(INDEX_TYPE_PARTEDCOMMON), i64(1), NULL_OBJ, i64(NULL_I64), NULL_OBJ,
2767-
clone_obj(filter), NULL_OBJ);
2768-
obj_p res = aggr_dev(val, index);
2769-
drop_obj(index);
2770-
return res;
2771-
}
2772-
obj_p collected = filter_collect(val, filter);
2773-
obj_p res = ray_dev(collected);
2774-
drop_obj(collected);
2775-
return res;
2776-
}
27772791
case TYPE_PARTEDI16:
27782792
case TYPE_PARTEDI32:
27792793
case TYPE_PARTEDI64:

‎core/update.c‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,27 @@ obj_p ray_upsert(obj_p *x, i64_t n) {
633633
k2 = clone_obj(AS_LIST(lst)[0]);
634634
} else {
635635
k1 = ray_take(AS_LIST(obj)[1], x[1]);
636-
k2 = ray_take(lst, x[1]);
636+
// Single-record atoms must be wrapped in single-element vectors
637+
// because index_upsert_obj compares via AS_I64(col)[row], which
638+
// reads past the struct header for atoms (garbage data).
639+
i64_t nkeys = x[1]->i64;
640+
k2 = LIST(nkeys);
641+
if (IS_ERR(k2)) {
642+
drop_obj(k1);
643+
UPSERT_ERROR(k2);
644+
}
645+
for (i64_t j = 0; j < nkeys; j++) {
646+
obj_p atom = AS_LIST(lst)[j];
647+
obj_p vec = vector(atom->type, 1);
648+
if (IS_ERR(vec)) {
649+
k2->len = j;
650+
drop_obj(k2);
651+
drop_obj(k1);
652+
UPSERT_ERROR(vec);
653+
}
654+
ins_obj(&vec, 0, clone_obj(atom));
655+
AS_LIST(k2)[j] = vec;
656+
}
637657
}
638658

639659
idx = index_upsert_obj(k2, k1, x[1]->i64);

‎tests/main.c‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -579,18 +579,49 @@ test_entry_t tests[] = {
579579
{"test_table_metadata", test_table_metadata},
580580
{"test_select_basic", test_select_basic},
581581
{"test_select_where", test_select_where},
582+
{"test_select_where_types", test_select_where_types},
582583
{"test_select_aggregation", test_select_aggregation},
583-
{"test_select_multikey", test_select_multikey},
584-
{"test_update_operations", test_update_operations},
585-
{"test_insert_upsert", test_insert_upsert},
584+
{"test_select_groupby_single", test_select_groupby_single},
585+
{"test_select_groupby_multi", test_select_groupby_multi},
586+
{"test_select_groupby_xbar", test_select_groupby_xbar},
587+
{"test_select_where_by", test_select_where_by},
588+
{"test_select_computed_columns", test_select_computed_columns},
589+
{"test_select_empty_tables", test_select_empty_tables},
590+
{"test_select_large_tables", test_select_large_tables},
591+
{"test_select_string_groupby", test_select_string_groupby},
592+
{"test_select_parallel_filter", test_select_parallel_filter},
593+
{"test_update_basic", test_update_basic},
594+
{"test_update_where", test_update_where},
595+
{"test_update_by", test_update_by},
596+
{"test_update_inplace", test_update_inplace},
597+
{"test_update_types", test_update_types},
598+
{"test_insert_basic", test_insert_basic},
599+
{"test_insert_inplace", test_insert_inplace},
600+
{"test_insert_types", test_insert_types},
601+
{"test_upsert_basic", test_upsert_basic},
602+
{"test_upsert_multi", test_upsert_multi},
603+
{"test_upsert_inplace", test_upsert_inplace},
604+
{"test_alter_vectors", test_alter_vectors},
605+
{"test_alter_tables", test_alter_tables},
586606
{"test_table_concat", test_table_concat},
587607
{"test_table_nulls", test_table_nulls},
588608
{"test_query_complex", test_query_complex},
589609
{"test_table_all_types", test_table_all_types},
590610
{"test_table_dict_conversion", test_table_dict_conversion},
591-
{"test_select_string_groupby", test_select_string_groupby},
592-
{"test_select_parallel_filter", test_select_parallel_filter},
593611
{"test_table_edge_cases", test_table_edge_cases},
612+
{"test_query_chains", test_query_chains},
613+
{"test_select_take", test_select_take},
614+
{"test_select_sorting", test_select_sorting},
615+
{"test_select_dev_med", test_select_dev_med},
616+
{"test_select_xbar_dates", test_select_xbar_dates},
617+
{"test_update_where_by", test_update_where_by},
618+
{"test_set_operations", test_set_operations},
619+
{"test_select_where_expr", test_select_where_expr},
620+
{"test_insert_edge_cases", test_insert_edge_cases},
621+
{"test_upsert_edge_cases", test_upsert_edge_cases},
622+
{"test_alter_edge_cases", test_alter_edge_cases},
623+
{"test_select_distinct", test_select_distinct},
624+
{"test_table_operations_mixed", test_table_operations_mixed},
594625
// Advanced: dict, lambda, iteration, error tests
595626
{"test_dict_advanced", test_dict_advanced},
596627
{"test_lambda_advanced", test_lambda_advanced},

0 commit comments

Comments
 (0)