perf(gfql): dtype-gate temporal-text detection to avoid spurious stringification (#1650)#1651
Closed
lmeyerov wants to merge 1 commit into
Closed
perf(gfql): dtype-gate temporal-text detection to avoid spurious stringification (#1650)#1651lmeyerov wants to merge 1 commit into
lmeyerov wants to merge 1 commit into
Conversation
…ngification (#1650) `order_detect_temporal_mode` ran `astype(str)` + up to 6 regex `fullmatch` passes on BOTH operands of every comparison, including numeric/bool columns that can never hold temporal *text*. This fired ~160k spurious `re.fullmatch` on a 10k-row `where_rows(val > 50)` whose output is a plain table — pure waste. Gate: early-return None when `dtype.kind in {i,u,f,b,c}`. Object/string/datetime columns are unaffected, so temporal-text detection is preserved. Byte-identical output. Measured `where_rows` speedups: - pandas @10k: 48.9 -> 15.6 ms (3.1x) - cuDF @10k: 37.4 -> 8.5 ms (4.4x) - cuDF @100k: 191.0 -> 14.4 ms (13.3x, 1M edges) Tests: pandas 91 + cuDF 83 temporal/ordering/row suites pass; adds unit coverage for the gate (numeric/bool skip, object temporal still detected) and an end-to-end numeric `where_rows` filter assertion. Scope: this is the spurious-path half of #1650. Whole-entity `RETURN a` text rendering (structured/Arrow returns) is tracked separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lmeyerov
added a commit
that referenced
this pull request
Jun 26, 2026
…on scans Follows #1650/#1651 (which gated temporal-text detection). Profiling numeric RETURN/ORDER BY queries @100k showed ~40–48% of the call was STILL spurious `astype(str)` + regex on int/float/bool columns — not in temporal detection (already gated) but in the LIST-detection + projection scans: - order_detect_list_series / order_detect_stringified_list_series (ordering.py) - RowPipelineMixin._gfql_series_is_list_like (pipeline.py) - _project_property_column temporal scan (result_postprocess.py) All run `series.astype(str)` to test for list/temporal *text* on every column, including numeric/bool/complex columns that can never hold it. Gate them with a shared `_is_non_listable_dtype` (kind in i,u,f,b,c) → early return False/series. Byte-identical (the scans return False/None for these dtypes anyway); object/ string/datetime columns are untouched, preserving list/temporal detection. Result (where the projection/order path runs over numeric cols): ORDER BY and arithmetic-projection drop their ~50–100ms spurious string render to ~0. + tests: numeric/bool dtype pass-through; behavioral numeric ORDER BY (would fail if values were lexically stringified). Base cypher/row code -> PR0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lmeyerov
added a commit
that referenced
this pull request
Jun 28, 2026
The #1650/#1651 dtype gate (numeric/bool/complex columns skip the spurious astype(str)+regex list/temporal scan) was duplicated across 4 sites with 3 different sentinel conventions ("O", None, factored). Consolidate the kind set into is_non_textual_scalar_dtype() in series_str_compat.py (the engine-poly Series-string layer these gates short-circuit) and call it from ordering.py (x2), pipeline.py, and cypher/result_postprocess.py. Byte-identical; adds a direct parametrized unit test for the helper across dtype kinds + None. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lmeyerov
added a commit
that referenced
this pull request
Jun 28, 2026
The #1650/#1651 dtype gate (numeric/bool/complex columns skip the spurious astype(str)+regex list/temporal scan) was duplicated across 4 sites with 3 different sentinel conventions ("O", None, factored). Consolidate the kind set into is_non_textual_scalar_dtype() in series_str_compat.py (the engine-poly Series-string layer these gates short-circuit) and call it from ordering.py (x2), pipeline.py, and cypher/result_postprocess.py. Byte-identical; adds a direct parametrized unit test for the helper across dtype kinds + None. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lmeyerov
added a commit
that referenced
this pull request
Jun 29, 2026
The #1650/#1651 dtype gate (numeric/bool/complex columns skip the spurious astype(str)+regex list/temporal scan) was duplicated across 4 sites with 3 different sentinel conventions ("O", None, factored). Consolidate the kind set into is_non_textual_scalar_dtype() in series_str_compat.py (the engine-poly Series-string layer these gates short-circuit) and call it from ordering.py (x2), pipeline.py, and cypher/result_postprocess.py. Byte-identical; adds a direct parametrized unit test for the helper across dtype kinds + None. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lmeyerov
added a commit
that referenced
this pull request
Jun 29, 2026
The #1650/#1651 dtype gate (numeric/bool/complex columns skip the spurious astype(str)+regex list/temporal scan) was duplicated across 4 sites with 3 different sentinel conventions ("O", None, factored). Consolidate the kind set into is_non_textual_scalar_dtype() in series_str_compat.py (the engine-poly Series-string layer these gates short-circuit) and call it from ordering.py (x2), pipeline.py, and cypher/result_postprocess.py. Byte-identical; adds a direct parametrized unit test for the helper across dtype kinds + None. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the spurious-stringification half of #1650. GFQL ran row-wise entity/temporal stringification in comparison paths whose output never contains entity-text (e.g.
where_rows). Root cause:order_detect_temporal_mode(ordering.py) runsastype(str)+ up to 6 regexfullmatchpasses on both operands of every comparison — includingint64/boolcolumns that can never hold temporal text. A 10k-rowwhere_rows(val > 50), which returns a plain 3-column table, fired ~160,006 spuriousre.fullmatchcalls, all discarded.Fix
order_detect_temporal_modeearly-returnsNonewhenseries.dtype.kind in {i, u, f, b, c}(numeric/bool/complex). Object/string/datetime columns are unaffected, so temporal-text detection is preserved.Results (byte-identical output, gated == ungated)
where_rows(val>50)Speedup scales with row count (spurious scan is O(rows)). cuDF measured on a GB10 box with
graphistry/test-gpu:latest(cudf 25.12).Tests
TEST_CUDF=1container): temporal + ordering + row suites — 83 passorder_detect_temporal_modeunit coverage (numeric/bool skip; object temporal still detected) + end-to-end numericwhere_rowsfilter assertion.🤖 Generated with Claude Code