Skip to content

[query] use lowered TableMapRows implementation #14957

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
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

[query] use lowered TableMapRows implementation #14957

wants to merge 1 commit into from

Conversation

@ehigham
Copy link
Member

@ehigham ehigham commented Jul 21, 2025 •
edited
Loading

Two of the cases in TableValue.mapRows were effectly redudant as they were implemented in single-branch if statements without an early return.
None of these cases were tested and one was hidden behind a feature flag.

This change adopts the lowered execution path for TableMapRows after cloud benchmarks found no measurable performance differences between implementations.

@ehigham ehigham requested a review from patrick-schultz July 21, 2025 19:36
patrick-schultz
patrick-schultz requested changes Jul 21, 2025
View reviewed changes
Copy link
Member

@patrick-schultz patrick-schultz left a comment

Choose a reason for hiding this comment

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

It's annoying, but since this exposes two different previously unused code paths, I'd feel better if we knew they're exercised by tests. Could you try adding an assert(false) to each and making sure there are failing tests? (Assuming no tests fail without that.)

@ehigham ehigham requested a review from chrisvittal July 21, 2025 20:21
@ehigham
Copy link
Member Author

ehigham commented Jul 21, 2025

Sure. I'm confident these are not covered by tests.

@ehigham Graphite App
Copy link
Member Author

ehigham commented Jul 21, 2025

We're exercising the first branch where there are no extracted aggregations. Will remove that assertion and see if the second one is hit

@ehigham ehigham force-pushed the ehigham/fix-table-value-map-rows branch from 3de8c05 to 1b6a93c Compare July 25, 2025 21:51
@ehigham
Copy link
Member Author

ehigham commented Jul 25, 2025 •
edited
Loading

Following the work concluded by #14762, benchmarks show no measurable differences in runtime between lowered and non-lowered implementation of TableMapRows with scans.
Below are the benchmark results:

path name changed relative_change
benchmark/hail/benchmark_benchmark_analysis.py benchmark_analyze_benchmarks FALSE {"start":0.8882488281239381,"end":1.1055864294144548,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_combiner.py benchmark_import_and_transform_gvcf FALSE {"start":0.9157581573245972,"end":1.060314419047487,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_combiner.py benchmark_import_gvcf_force_count FALSE {"start":0.8749196884725537,"end":1.166278317862579,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_combiner.py benchmark_python_only_10k_combine FALSE {"start":0.9250412394834756,"end":1.069458463235141,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_linalg.py benchmark_blockmatrix_write_from_entry_expr_range_mt_standardize FALSE {"start":"NaN","end":"NaN","includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_linalg.py benchmark_make_ndarray FALSE {"start":0.8835159124372942,"end":1.096395461441493,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_linalg.py benchmark_ndarray_addition FALSE {"start":0.8816313518143155,"end":1.0404970331588093,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_linalg.py benchmark_ndarray_matmul_float64 FALSE {"start":0.8850537518568584,"end":1.1012344256504458,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_linalg.py benchmark_ndarray_matmul_int64 FALSE {"start":0.8839788013120572,"end":1.083609441329138,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_linalg.py benchmark_sum_table_of_ndarrays FALSE {"start":"NaN","end":"NaN","includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_export_range_matrix_table_col_p100 FALSE {"start":0.7916378827620718,"end":0.992916329726392,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_export_range_matrix_table_entry_field_p100 FALSE {"start":0.9591822383744986,"end":1.137465787629,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_export_range_matrix_table_row_p100 TRUE {"start":0.6604507817182838,"end":0.9023749438555168,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_gnomad_coverage_stats FALSE {"start":0.9429604148579873,"end":1.0656479066513707,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_gnomad_coverage_stats_optimized FALSE {"start":0.9538113874206581,"end":1.0253483709949789,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_import_bgen_filter_count FALSE {"start":0.8766765174768791,"end":1.0059573477120205,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_import_bgen_force_count_just_gp FALSE {"start":0.9650292866399206,"end":1.0606140671265352,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_kyle_sex_specific_qc FALSE {"start":0.9615676810706509,"end":1.0493347180731698,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_aggregate_entries FALSE {"start":0.8905330299961929,"end":0.9982892209500898,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_array_arithmetic FALSE {"start":0.9498414381728323,"end":1.0237701118914107,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_call_stats_star_star FALSE {"start":0.9607727191439586,"end":1.1007361271890834,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_cols_show FALSE {"start":0.9513626503454327,"end":1.1906700833737378,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_decode_and_count FALSE {"start":0.9329830732425405,"end":1.033797941099407,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_decode_and_count_just_gt FALSE {"start":0.9219114789061069,"end":1.0829560137860281,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_entries_show FALSE {"start":0.8698940446732586,"end":1.0526416888890973,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_entries_table FALSE {"start":0.9514231309937509,"end":1.0534886458013804,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_entries_table_no_key FALSE {"start":0.9163175618306154,"end":1.0289607343244722,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_filter_entries FALSE {"start":0.9490379737668526,"end":1.0547155483503712,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_filter_entries_unfilter FALSE {"start":0.9030390335489886,"end":1.046321588902374,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_many_aggs_col_wise FALSE {"start":0.9450261599968999,"end":1.0572460980053253,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_many_aggs_row_wise FALSE {"start":0.8913504353316798,"end":1.0304931472143588,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_rows_force_count FALSE {"start":0.8249466474778303,"end":1.0658480887247668,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_rows_is_transition FALSE {"start":0.913506875376051,"end":1.1373676402821833,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_rows_show FALSE {"start":0.9177202133472983,"end":1.1028485025000652,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_scan_count_cols_2 FALSE {"start":0.7442634920979707,"end":1.0092347272166224,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_scan_count_rows_2 FALSE {"start":0.8827653539797808,"end":0.9885026004288615,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_show FALSE {"start":0.8858625790082815,"end":1.0862049448096156,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_take_col FALSE {"start":0.8656039873834955,"end":1.1227807953228088,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_take_entry FALSE {"start":0.8805245779070082,"end":1.0655828377766818,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_matrix_table_take_row FALSE {"start":0.8906533444021758,"end":1.1217985625026379,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_mt_localize_and_collect FALSE {"start":0.9297931698279883,"end":1.115650627423812,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_per_row_stats_star_star FALSE {"start":0.9059392329577918,"end":1.0262259641115412,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_read_decode_gnomad_coverage FALSE {"start":0.9334098224771229,"end":1.0491928093030343,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_write_profile_mt FALSE {"start":0.9307902353681287,"end":1.0351905196126823,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_matrix_table.py benchmark_write_range_matrix_table_p100 FALSE {"start":0.8283379441032411,"end":0.9906973656868153,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_concordance FALSE {"start":0.8407387754356513,"end":1.0066545911036322,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_export_vcf FALSE {"start":0.9194504444074824,"end":1.0325650897971794,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_genetics_pipeline FALSE {"start":0.9347335936020984,"end":1.0225540434415115,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_hwe_normalized_pca TRUE {"start":1.2190218151388625,"end":1.312299547225575,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_hwe_normalized_pca_blanczos_small_data_0_iterations FALSE {"start":0.8959146954629177,"end":1.0632547839356126,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_hwe_normalized_pca_blanczos_small_data_10_iterations FALSE {"start":0.8868775463130633,"end":1.0168394172445627,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_import_vcf_count_rows FALSE {"start":0.8688477579892161,"end":1.0689485782790165,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_import_vcf_write FALSE {"start":0.9557435402321581,"end":1.0447076185680675,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_pc_relate FALSE {"start":0.9700937061139713,"end":1.042301093207549,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_sample_qc FALSE {"start":0.871614361071837,"end":1.0176909024650416,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_split_multi FALSE {"start":0.9316969799738914,"end":1.170092605177858,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_split_multi_hts FALSE {"start":0.9267955018643608,"end":1.0184928460594644,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_variant_and_sample_qc FALSE {"start":0.8833005239268064,"end":1.0029396222811293,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_variant_and_sample_qc_nested_with_filters_2 FALSE {"start":0.9074319400378986,"end":1.0256263184543526,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_variant_and_sample_qc_nested_with_filters_4 FALSE {"start":0.9791507335251726,"end":1.0384627810723146,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_variant_and_sample_qc_nested_with_filters_4_counts FALSE {"start":0.966554584770333,"end":1.0241183136684573,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_methods.py benchmark_variant_qc FALSE {"start":0.9460056417224377,"end":1.0383746535841334,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_sentinel.py benchmark_sentinel_cpu_hash_1 FALSE {"start":0.9280541277895645,"end":1.0120087863068077,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_sentinel.py benchmark_sentinel_read_gunzip FALSE {"start":0.9607615945033844,"end":1.0693661941106343,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_shuffle.py benchmark_shuffle_key_by_aggregate_bad_locality FALSE {"start":0.9271885841083493,"end":1.0131959466241915,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_shuffle.py benchmark_shuffle_key_by_aggregate_good_locality FALSE {"start":0.9935509875630941,"end":1.063377075111227,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_shuffle.py benchmark_shuffle_key_rows_by_4096_byte_rows FALSE {"start":0.9016418806460457,"end":1.069526016465493,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_shuffle.py benchmark_shuffle_key_rows_by_65k_byte_rows FALSE {"start":0.9360368909181859,"end":1.1141057340889,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_shuffle.py benchmark_shuffle_key_rows_by_mt FALSE {"start":0.9324506886370504,"end":1.0529489806532184,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_shuffle.py benchmark_shuffle_order_by_10m_int FALSE {"start":0.9321374090211315,"end":1.0718594506253785,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_group_by_collect_per_row FALSE {"start":0.8902000603166607,"end":1.0116729248203655,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_group_by_take_rekey FALSE {"start":0.8699012332342841,"end":1.0619979408740912,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[10-1000] FALSE {"start":0.8824806785491223,"end":1.0031400874137713,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[10-100] FALSE {"start":0.9026816194426142,"end":0.9983842891308881,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[10-10] FALSE {"start":0.8954633263509414,"end":1.1114815438547534,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[100-1000] FALSE {"start":0.9297701792388312,"end":1.0649144627785923,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[100-100] FALSE {"start":0.9012143703126875,"end":1.0348209365736485,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[100-10] FALSE {"start":0.8834081243628931,"end":1.0151780014757297,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[1000-1000] FALSE {"start":0.8866322782683016,"end":1.0465156226175059,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[1000-100] FALSE {"start":0.9461889268682309,"end":1.097230506915925,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_join_partitions_table[1000-10] FALSE {"start":0.8925943296441833,"end":1.0183188614603182,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_read_force_count_partitions[1000] FALSE {"start":0.8735394683085057,"end":1.0368027281700114,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_read_force_count_partitions[100] FALSE {"start":0.8959323523188781,"end":1.1129332620838195,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_read_force_count_partitions[10] FALSE {"start":0.8297438031081406,"end":0.9943830526245139,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_read_with_index[1000] FALSE {"start":0.9082348307056985,"end":1.1135010436317303,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_aggregate_array_sum FALSE {"start":0.9177346627573916,"end":1.0093100788875764,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_aggregate_counter FALSE {"start":0.8792352775552931,"end":1.021319523953299,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_aggregate_downsample_dense FALSE {"start":0.9175970125264624,"end":1.0341287713178196,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_aggregate_downsample_worst_case FALSE {"start":0.8636183080399854,"end":1.0116246607763408,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_aggregate_int_stats FALSE {"start":0.9604567587034775,"end":1.0508541156410987,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_aggregate_linreg FALSE {"start":0.940845406314315,"end":1.0507016109851584,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_aggregate_take_by_strings FALSE {"start":0.8999808113912935,"end":1.041388525718002,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_annotate_many_flat FALSE {"start":0.9174658724133599,"end":1.0267968060425514,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_big_aggregate_compilation FALSE {"start":0.9698298295776311,"end":1.0270525487606097,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_big_aggregate_compile_and_execute FALSE {"start":0.9163475894825364,"end":1.0107842290796123,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_expr_take FALSE {"start":0.8609061384624799,"end":1.0494796665920838,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_foreign_key_join[1000000-1000000] FALSE {"start":0.9562661046446795,"end":1.0737292422428175,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_foreign_key_join[1000000-1000] FALSE {"start":0.8700911728403395,"end":0.9901967771814572,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_group_by_aggregate_sorted TRUE {"start":0.7848976052336071,"end":0.9287922873810349,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_group_by_aggregate_unsorted FALSE {"start":0.9183470537675824,"end":1.0896598807071747,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_import_ints FALSE {"start":0.9185893906620819,"end":1.0622172643962118,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_import_ints_impute FALSE {"start":0.9474863189401723,"end":1.0751321033481145,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_import_strings FALSE {"start":0.9411955246846888,"end":1.0518481591263154,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_key_by_shuffle FALSE {"start":0.8841422729853657,"end":1.0626865980511377,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_python_construction FALSE {"start":0.8855162823186105,"end":1.1590875545700934,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_range_array_range_force_count FALSE {"start":0.9296475376859693,"end":1.0681384155676827,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_range_force_count FALSE {"start":0.9358065604235674,"end":1.041767165697597,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_range_join[1000000000-1000000000] FALSE {"start":0.9679024770850326,"end":1.0696268197966337,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_range_join[1000000000-1000] FALSE {"start":0.9096692731084839,"end":1.0942245172931924,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_range_means FALSE {"start":0.9410161202172702,"end":1.1467493268854012,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_read_force_count_ints FALSE {"start":0.9294988474328767,"end":1.0377460423545941,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_read_force_count_strings FALSE {"start":0.9302287344950569,"end":1.1004511780471384,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_scan_prev_non_null FALSE {"start":0.9738795831334156,"end":1.0982830520600395,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_scan_sum_1k_partitions FALSE {"start":0.8771378326982002,"end":1.0442921588778147,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_show FALSE {"start":0.9160504875502237,"end":1.0655442289153365,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_table_take FALSE {"start":0.8010992546778521,"end":1.0254717198848011,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_test_head_and_tail_region_memory FALSE {"start":0.8134618138738137,"end":0.9945566330027368,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_test_inner_join_region_memory TRUE {"start":0.735187527383166,"end":0.9117269684501581,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_test_left_join_region_memory FALSE {"start":0.8236058741018464,"end":0.9984485304094747,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_test_map_filter_region_memory TRUE {"start":0.8534625192992172,"end":0.963896422572966,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[10-1000] FALSE {"start":0.9917316111682934,"end":1.2003087064708908,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[10-100] FALSE {"start":0.9216747614316169,"end":1.0617707148659972,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[10-10] TRUE {"start":0.7469193119351658,"end":0.9531814918066145,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[100-1000] FALSE {"start":0.9516715871830465,"end":1.0352367956432897,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[100-100] FALSE {"start":0.8906198007919243,"end":1.090679575801985,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[100-10] FALSE {"start":0.9441419398701483,"end":1.0802359756387114,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[1000-1000] FALSE {"start":0.9406256152940027,"end":1.1209730998555187,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[1000-100] FALSE {"start":0.8939055723118319,"end":1.0049082968480083,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_union_partitions_table[1000-10] FALSE {"start":0.9859239773602916,"end":1.1336259221508642,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_write_range_table[10000000-1000] FALSE {"start":0.8652360625153536,"end":1.1656865011965303,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_write_range_table[10000000-100] FALSE {"start":0.8400529273761284,"end":0.9849723829167591,"includeStart":true,"includeEnd":true}
benchmark/hail/benchmark_table.py benchmark_write_range_table[10000000-10] FALSE {"start":0.9212875281054026,"end":1.0687986220639805,"includeStart":true,"includeEnd":true}

@ehigham ehigham requested a review from patrick-schultz July 25, 2025 22:04
@ehigham ehigham changed the title [query] fix TableValue.mapRows ignored branches [query] use lowered TableMapRows implementation Jul 25, 2025
patrick-schultz
patrick-schultz requested changes Jul 28, 2025
View reviewed changes
Copy link
Member

@patrick-schultz patrick-schultz left a comment

Choose a reason for hiding this comment

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

Very excited to be deleting some old unlowered implementations!

hail/hail/src/is/hail/expr/ir/lowering/ExecuteRelational.scala
val extracted = agg.Extract(newRow, r.requirednessAnalysis, isScan = true)
TableValueIntermediate(recur(child).asTableValue(ctx).mapRows(extracted))
case ir: TableMapRows =>
TableStageIntermediate(ctx.backend.tableToTableStage(ctx, ir, r))
Copy link
Member

Choose a reason for hiding this comment

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

This forces the entire child pipeline to be lowered, which isn't what we want here. See the TableJoin case as an example; we want to factor out the TableMapRows lowering implementation to be called directly from both the table lowering pass and here.

@ehigham Graphite App
Copy link
Member Author

ehigham commented Jul 28, 2025

The test failures make me slightly nervous that the table benchmarks don't actually exercise the map rows for scans at all.

@ehigham ehigham marked this pull request as draft September 25, 2025 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@patrick-schultz patrick-schultz patrick-schultz requested changes

@chrisvittal chrisvittal Awaiting requested review from chrisvittal

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ehigham @patrick-schultz