Fix LOGICAL_ERROR when ROW POLICY is used with ALIAS column containing dictGet by fastio · Pull Request #99065 · ClickHouse/ClickHouse

fastio · 2026-03-09T13:04:38Z

Changelog category (leave one):

Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fix LOGICAL_ERROR when querying a table that has both a ROW POLICY and an ALIAS column using dictGet. The issue was caused by premature access to the table expression during ALIAS column resolution in the
new analyzer.

Fixes #94659

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

No documentation changes required. This is a bug fix for an internal error (LOGICAL_ERROR) that occurred when a table with an ALIAS column referencing dictGet was queried while a ROW POLICY was active on that
table.

#94659

Note

Medium Risk
Touches core query analyzer resolution flow by adding resolve-in-progress guarding around initializeTableExpressionData, which could affect identifier resolution for some queries. Change is small but in a central code path and validated by a targeted regression test.

Overview
Fixes a new-analyzer LOGICAL_ERROR when a table has both a ROW POLICY and an ALIAS column using dictGet by marking the current table_expression as in resolve while running initializeTableExpressionData, preventing premature access during ALIAS column resolution.

Adds a stateless regression test (04033_issue_94659) covering selects with/without the ALIAS column, with an active row policy, and ensuring the old analyzer path still works.

^{Written by Cursor Bugbot for commit 927f83a. This will update automatically on new commits. Configure here.}

…g dictGet

clickhouse-gh · 2026-03-09T13:31:58Z

Workflow [PR], commit [8055fda]

Summary: ✅

Copilot

Pull request overview

Fixes a new-analyzer LOGICAL_ERROR triggered when querying a table that has an active ROW POLICY and an ALIAS column using dictGet, by preventing premature access to partially-initialized table-expression data during single-expression resolution.

Changes:

Add a “table expression is in resolve” guard around initializeTableExpressionData() in QueryAnalyzer::resolve() for expression nodes.
Add a stateless regression test reproducing ROW POLICY + dictGet ALIAS behavior (including an enable_analyzer=0 control query).
Add expected output reference for the new test.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
`src/Analyzer/Resolve/QueryAnalyzer.cpp`	Adds a guard in `QueryAnalyzer::resolve()` to avoid premature table-expression access during ALIAS resolution.
`tests/queries/0_stateless/04033_issue_94659.sql`	New regression test reproducing the crash scenario and validating both analyzers.
`tests/queries/0_stateless/04033_issue_94659.reference`	Expected output for the new stateless test.

You can also share your feedback on Copilot code review. Take the survey.

src/Analyzer/Resolve/QueryAnalyzer.cpp

Copilot · 2026-03-09T13:50:06Z

tests/queries/0_stateless/04033_issue_94659.sql

+-- Tags: no-parallel
+-- Issue: https://github.com/ClickHouse/ClickHouse/issues/94659
+-- ROW POLICY + ALIAS column with dictGet should not cause LOGICAL_ERROR
+
+DROP ROW POLICY IF EXISTS pol_94659 ON t1_94659;
+DROP TABLE IF EXISTS t1_94659;
+DROP DICTIONARY IF EXISTS d1_94659;
+DROP TABLE IF EXISTS t2_94659;


This regression test relies on whatever the default enable_analyzer value is for the test runner. To ensure it actually exercises the new analyzer path that used to crash, please add an explicit SET enable_analyzer = 1; near the top (and keep the enable_analyzer = 0 query as-is for the old-analyzer check).

This is a useful tip for sure.

Yes, I have fixed it.

clickhouse-gh · 2026-03-13T03:56:43Z

tests/queries/0_stateless/04033_issue_94659.sql

@@ -0,0 +1,53 @@
+-- Tags: no-parallel


-- Tags: no-parallel looks unnecessary here because all object names are uniquely suffixed (*_94659), so this test should be isolated and parallel-safe. Keeping no-parallel reduces CI throughput; please remove the tag unless there is a reproducible parallel conflict that requires it.

I guess no-parallel is because of the row policy, right?

I checked the ROW POLICY test cases; there’s no need to add the no-parallel tag. I remove it now.

fastio · 2026-03-15T09:15:57Z

Hi @antaljanosbenjamin, Could you please review this PR when you have a chance?

antaljanosbenjamin · 2026-03-18T10:34:49Z

tests/queries/0_stateless/04033_issue_94659.sql

@@ -0,0 +1,53 @@
+-- Tags: no-parallel


I guess no-parallel is because of the row policy, right?

clickhouse-gh · 2026-03-18T18:04:21Z

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	83.80%	83.70%	-0.10%
Functions	23.90%	23.90%	+0.00%
Branches	76.40%	76.30%	-0.10%

PR changed lines: PR changed-lines coverage: 100.00% (18/18, 0 noise lines excluded)
Diff coverage report
Uncovered code

fastio · 2026-03-19T00:33:22Z

@antaljanosbenjamin Hi, -- Tags : no-parallel is not required. I've removed it.

antaljanosbenjamin · 2026-03-19T08:38:21Z

Thanks for the great work and sorry for dragging the review, I have quite a packed few days (weeks?) behind me.

Fix LOGICAL_ERROR when ROW POLICY is used with ALIAS column containin…

289cd48

…g dictGet

bharatnc added the can be tested Allows running workflows for external contributors label Mar 9, 2026

clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label Mar 9, 2026

antaljanosbenjamin self-assigned this Mar 9, 2026

antaljanosbenjamin requested a review from Copilot March 9, 2026 13:45

Copilot started reviewing on behalf of antaljanosbenjamin March 9, 2026 13:46 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

fastio added 3 commits March 10, 2026 20:47

fix test

fe56c5a

Merge branch 'ClickHouse:master' into bugfix-94659

12de695

Merge branch 'ClickHouse:master' into bugfix-94659

927f83a

clickhouse-gh bot reviewed Mar 13, 2026

View reviewed changes

antaljanosbenjamin approved these changes Mar 18, 2026

View reviewed changes

fix tests

8055fda

antaljanosbenjamin added this pull request to the merge queue Mar 19, 2026

antaljanosbenjamin approved these changes Mar 19, 2026

View reviewed changes

Merged via the queue into ClickHouse:master with commit a747d8a Mar 19, 2026
163 checks passed

robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 19, 2026

Conversation

fastio commented Mar 9, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Documentation entry for user-facing changes

Uh oh!

clickhouse-gh bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

antaljanosbenjamin Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

fastio Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh bot Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

antaljanosbenjamin Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

fastio Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

fastio commented Mar 15, 2026

Uh oh!

antaljanosbenjamin Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh bot commented Mar 18, 2026

LLVM Coverage Report

Uh oh!

fastio commented Mar 19, 2026

Uh oh!

antaljanosbenjamin commented Mar 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fastio commented Mar 9, 2026 •

edited by cursor bot

Loading

clickhouse-gh bot commented Mar 9, 2026 •

edited

Loading