Skip to content

Conversation

tobixdev
Copy link
Contributor

@tobixdev tobixdev commented Oct 3, 2025

Which issue does this PR close?

Related to #17904

If the outcome of the discussion is that this optimization makes sense, we can improve the code.

Rationale for this change

Should provide more context to the discussion in the ticket.

What changes are included in this PR?

The optimization pass in the ticket.

Are these changes tested?

Yes there are unit tests. It is also covered by the regular test suite where changes are correctly (not) introduced.

Are there any user-facing changes?

Their execution plans could change.

@tobixdev
Copy link
Contributor Author

tobixdev commented Oct 3, 2025

@alamb Could you please run the TPC-H benchmark? There are some changes to the execution plan. Locally, I didn't observe any improvements but just to make sure and provide further context to the discussion in #17904 .

Thanks!

@alamb
Copy link
Contributor

alamb commented Oct 3, 2025

🤖 ./gh_compare_branch.sh Benchmark Script Running
Linux aal-dev 6.14.0-1016-gcp #17~24.04.1-Ubuntu SMP Wed Sep 3 01:55:36 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing feature/nl-join-projection-push-down (effc881) to 71512e6 diff using: tpch_mem
Results will be posted here when complete

@alamb
Copy link
Contributor

alamb commented Oct 3, 2025

🤖: Benchmark completed

Details

Comparing HEAD and feature_nl-join-projection-push-down
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃      HEAD ┃ feature_nl-join-projection-push-down ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 1     │ 173.94 ms │                            166.79 ms │     no change │
│ QQuery 2     │  26.84 ms │                             26.31 ms │     no change │
│ QQuery 3     │  38.02 ms │                             40.60 ms │  1.07x slower │
│ QQuery 4     │  28.73 ms │                             27.55 ms │     no change │
│ QQuery 5     │  79.91 ms │                             75.58 ms │ +1.06x faster │
│ QQuery 6     │  19.36 ms │                             19.55 ms │     no change │
│ QQuery 7     │ 213.76 ms │                            211.23 ms │     no change │
│ QQuery 8     │  32.90 ms │                             32.51 ms │     no change │
│ QQuery 9     │ 101.88 ms │                             98.57 ms │     no change │
│ QQuery 10    │  58.94 ms │                             59.39 ms │     no change │
│ QQuery 11    │  16.78 ms │                             16.87 ms │     no change │
│ QQuery 12    │  50.52 ms │                             50.51 ms │     no change │
│ QQuery 13    │  46.21 ms │                             48.33 ms │     no change │
│ QQuery 14    │  13.74 ms │                             13.68 ms │     no change │
│ QQuery 15    │  23.94 ms │                             24.43 ms │     no change │
│ QQuery 16    │  24.24 ms │                             25.06 ms │     no change │
│ QQuery 17    │ 144.48 ms │                            150.15 ms │     no change │
│ QQuery 18    │ 320.54 ms │                            322.16 ms │     no change │
│ QQuery 19    │  35.55 ms │                             36.64 ms │     no change │
│ QQuery 20    │  48.23 ms │                             51.32 ms │  1.06x slower │
│ QQuery 21    │ 370.56 ms │                            336.51 ms │ +1.10x faster │
│ QQuery 22    │  24.33 ms │                             20.49 ms │ +1.19x faster │
└──────────────┴───────────┴──────────────────────────────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary                                   ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)                                   │ 1893.40ms │
│ Total Time (feature_nl-join-projection-push-down)   │ 1854.23ms │
│ Average Time (HEAD)                                 │   86.06ms │
│ Average Time (feature_nl-join-projection-push-down) │   84.28ms │
│ Queries Faster                                      │         3 │
│ Queries Slower                                      │         2 │
│ Queries with No Change                              │        17 │
│ Queries with Failure                                │         0 │
└─────────────────────────────────────────────────────┴───────────┘

@alamb
Copy link
Contributor

alamb commented Oct 3, 2025

🤖 ./gh_compare_branch.sh Benchmark Script Running
Linux aal-dev 6.14.0-1016-gcp #17~24.04.1-Ubuntu SMP Wed Sep 3 01:55:36 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing feature/nl-join-projection-push-down (effc881) to 71512e6 diff using: tpch_mem10
Results will be posted here when complete

@alamb
Copy link
Contributor

alamb commented Oct 3, 2025

🤖: Benchmark completed

Details

Comparing HEAD and feature_nl-join-projection-push-down
--------------------
Benchmark tpch_mem_sf10.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ Query        ┃ HEAD ┃ feature_nl-join-projection-push-down ┃       Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━╇━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━━┩
│ QQuery 1     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 2     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 3     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 4     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 5     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 6     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 7     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 8     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 9     │ FAIL │                                 FAIL │ incomparable │
│ QQuery 10    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 11    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 12    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 13    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 14    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 15    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 16    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 17    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 18    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 19    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 20    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 21    │ FAIL │                                 FAIL │ incomparable │
│ QQuery 22    │ FAIL │                                 FAIL │ incomparable │
└──────────────┴──────┴──────────────────────────────────────┴──────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━┓
┃ Benchmark Summary                                   ┃        ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━┩
│ Total Time (HEAD)                                   │ 0.00ms │
│ Total Time (feature_nl-join-projection-push-down)   │ 0.00ms │
│ Average Time (HEAD)                                 │ 0.00ms │
│ Average Time (feature_nl-join-projection-push-down) │ 0.00ms │
│ Queries Faster                                      │      0 │
│ Queries Slower                                      │      0 │
│ Queries with No Change                              │      0 │
│ Queries with Failure                                │     22 │
└─────────────────────────────────────────────────────┴────────┘

@tobixdev
Copy link
Contributor Author

tobixdev commented Oct 5, 2025

🤖: Benchmark completed
Details

@alamb Do you think the tpch_mem10 fail was a spurious failure? Or is there some indication that this is a problem with the code?

@tobixdev tobixdev force-pushed the feature/nl-join-projection-push-down branch from effc881 to 61999f3 Compare October 5, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants