Skip to content

Fix THERE_IS_NO_COLUMN for parallel replicas with constant table ALIAS#77307

Merged
KochetovNicolai merged 7 commits intomasterfrom
test-constant-alias-fix
Mar 11, 2025
Merged

Fix THERE_IS_NO_COLUMN for parallel replicas with constant table ALIAS#77307
KochetovNicolai merged 7 commits intomasterfrom
test-constant-alias-fix

Conversation

@KochetovNicolai
Copy link
Member

@KochetovNicolai KochetovNicolai commented Mar 7, 2025

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 to CHANGELOG.md):

Reverted

Fix the queries that read the ALIAS column from a table if the ALIAS expression is constant. Fix THERE_IS_NO_COLUMN for queries with parallel replicas enabled, and Number of columns doesn't match for distributed queries.

closes #76925
different fix for #77210

@clickhouse-gh
Copy link
Contributor

clickhouse-gh bot commented Mar 7, 2025

Workflow [PR], commit [fe8736a]

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Mar 7, 2025
@KochetovNicolai KochetovNicolai changed the title Test constants in aliases. Fix THERE_IS_NO_COLUMN for parallel replicas which uses constant table ALIAS expression Mar 7, 2025
@KochetovNicolai KochetovNicolai changed the title Fix THERE_IS_NO_COLUMN for parallel replicas which uses constant table ALIAS expression Fix THERE_IS_NO_COLUMN for parallel replicas with constant table ALIAS expression Mar 7, 2025
@KochetovNicolai KochetovNicolai changed the title Fix THERE_IS_NO_COLUMN for parallel replicas with constant table ALIAS expression Fix THERE_IS_NO_COLUMN for parallel replicas with constant table ALIAS Mar 7, 2025
@clickhouse-gh clickhouse-gh bot added pr-bugfix Pull request with bugfix, not backported by default and removed pr-not-for-changelog This PR should not be mentioned in the changelog labels Mar 7, 2025
@KochetovNicolai KochetovNicolai marked this pull request as ready for review March 9, 2025 11:53
@novikd novikd self-assigned this Mar 10, 2025
Copy link
Member

@novikd novikd left a comment

Choose a reason for hiding this comment

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

LGTM. Not very obvious code, requires adding good comments.

SET allow_experimental_parallel_reading_from_replicas = 1;
SET cluster_for_parallel_replicas = 'parallel_replicas';
DROP TABLE IF EXISTS test_table;
CREATE TABLE test_table (a UInt64, b UInt64, c UInt64, d UInt64, x Array(String))
Copy link
Member

Choose a reason for hiding this comment

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

Let's also add a test with Nested.

Comment on lines +678 to +685
if (column_node.hasExpression())
{
auto expression = column_node.getExpression();
if (expression->getNodeType() == QueryTreeNodeType::CONSTANT)
return visitConstant(expression);
else if (!use_column_identifier_as_action_node_name)
return visitImpl(expression);
}
Copy link
Member

Choose a reason for hiding this comment

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

Please, add a comment with an explanation.

@KochetovNicolai KochetovNicolai added this pull request to the merge queue Mar 11, 2025
Merged via the queue into master with commit bc5df49 Mar 11, 2025
119 of 126 checks passed
@KochetovNicolai KochetovNicolai deleted the test-constant-alias-fix branch March 11, 2025 18:18
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Mar 11, 2025
@Algunenano
Copy link
Member

The fuzzer of both this PR and now master is full of LOGICAL_ERRORS:

One stacktrace:

/tmp $ grep 'Sorting column' server.log
2025.03.11 20:40:34.937863 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Fatal> : Logical error: 'Sorting column __table1.y wasn't found in the ActionsDAG's outputs. DAG:
/tmp $ grep ca593eb4-598f-4c3f-a3ee-4279def24d42 server.log
2025.03.11 20:40:34.936264 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Debug> executeQuery: (from [::ffff:127.0.0.1]:43216) (query 8, line 10) SELECT y FROM test_table ORDER BY 1 ASC NULLS FIRST, c ASC NULLS LAST (stage: Complete)
2025.03.11 20:40:34.936616 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> Planner: Query to stage Complete
2025.03.11 20:40:34.936748 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> Planner: Query to stage WithMergeableState only analyze
2025.03.11 20:40:34.936822 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> Planner: Query from stage FetchColumns to stage WithMergeableState only analyze
2025.03.11 20:40:34.936909 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> SortingStep: Adjusting memory limit before external sort with 22.89 GiB (ratio: 0.5, available system memory: 45.78 GiB)
2025.03.11 20:40:34.936995 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Debug> executeQueryWithParallelReplicas: Executing read from default.test_table (9628409a-5000-41fb-80e1-e2d328d0c39b), header _CAST(['qwqw']_Array(String), 'Array(String)'_String) Array(String) Const(size = 0, Array(size = 1, UInt64(size = 1), String(size = 1))), __table1.c UInt64 UInt64(size = 0), query (SELECT __table1.y AS y FROM default.test_table AS __table1 ORDER BY __table1.y ASC NULLS FIRST, __table1.c ASC NULLS LAST), stage WithMergeableState with parallel replicas
2025.03.11 20:40:34.937011 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Information> executeQueryWithParallelReplicas: Disabling 'use_hedged_requests' in favor of 'enable_parallel_replicas'. Hedged connections are not used for parallel reading from replicas
2025.03.11 20:40:34.937025 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Information> ReadFromParallelRemoteReplicasStep: The number of replicas requested (1000) is bigger than the real number available in the cluster (11). Will use the latter number to execute the query.
2025.03.11 20:40:34.937311 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> Planner: Query to stage WithMergeableState
2025.03.11 20:40:34.937408 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> Planner: Query from stage FetchColumns to stage WithMergeableState
2025.03.11 20:40:34.937465 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> SortingStep: Adjusting memory limit before external sort with 22.89 GiB (ratio: 0.5, available system memory: 45.78 GiB)
2025.03.11 20:40:34.937539 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Debug> executeQueryWithParallelReplicas: Local replica got replica number 7
2025.03.11 20:40:34.937597 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> Planner: Query to stage WithMergeableState only analyze
2025.03.11 20:40:34.937661 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> Planner: Query from stage FetchColumns to stage WithMergeableState only analyze
2025.03.11 20:40:34.937713 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> SortingStep: Adjusting memory limit before external sort with 22.89 GiB (ratio: 0.5, available system memory: 45.78 GiB)
2025.03.11 20:40:34.937736 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Trace> Planner: Query from stage WithMergeableState to stage Complete
2025.03.11 20:40:34.937863 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Fatal> : Logical error: 'Sorting column __table1.y wasn't found in the ActionsDAG's outputs. DAG:
2025.03.11 20:40:34.958455 [ 180 ] {ca593eb4-598f-4c3f-a3ee-4279def24d42} <Fatal> : Stack trace (when copying this message, always include the lines below):
2025.03.11 20:44:06.269805 [ 1498 ] {} <Fatal> BaseDaemon: (version 25.3.1.1757 (official build), build id: 3E834C8B6ABF76BC77C4DF9854EE459DB844EE3A, git hash: bc5df492a7eea4bcff4920440cd6f07efdddb612) (from thread 180) (query_id: ca593eb4-598f-4c3f-a3ee-4279def24d42) (query: SELECT y FROM test_table ORDER BY 1 ASC NULLS FIRST, c ASC NULLS LAST) Received signal Aborted (6)

@KochetovNicolai please have a look

@Algunenano
Copy link
Member

Even more issues found -> #77558

Reverting

Algunenano added a commit to Algunenano/ClickHouse that referenced this pull request Mar 13, 2025
…tant-alias-fix"

This reverts commit bc5df49, reversing
changes made to 1abb692.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Logical Error: Error occurred while querying with parallel replicas

4 participants