Skip to content

Conversation

@maxjustus
Copy link

Fixes client throwing errors like Multiple entries with same key: number=1 and number=0 For queries with duplicate column names. IE: select number, * from system.numbers limit 1;. Aligns with clickhouse client which allows duplicate column names.

❯ clickhouse client --query "select number, * from system.numbers limit 1" --format JSONEachRow;                        08:33:45
{"number":0,"number":0}

Fixes client throwing errors like `Multiple entries with same key: number=1 and number=0` For queries with duplicate column names. IE: `select number, * from system.numbers limit 1;`
Copy link

@windsurf-bot windsurf-bot bot left a comment

Choose a reason for hiding this comment

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

💡 To request another review, post a new comment with "/windsurf-review".

colIndexMapBuilder.put(columnName, i);
}
this.colIndex = colIndexMapBuilder.build();
this.colIndex = colIndexMapBuilder;
Copy link

Choose a reason for hiding this comment

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

The colIndex map is no longer immutable after switching from ImmutableMap to HashMap, but it's still exposed as a public field. Consider making this field private or ensuring it remains immutable by wrapping it with Collections.unmodifiableMap() to prevent external modifications.

this.query = query;
this.columns = ImmutableList.copyOf(columns);
ImmutableMap.Builder<String, Integer> colIndexMapBuilder = ImmutableMap.builder();
Map<String, Integer> colIndexMapBuilder = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

HashMap is much slower that ImmutableMap - that we have measured in our benchmark tests.
ImmutableMap allows to leave last value.

Copy link
Author

Choose a reason for hiding this comment

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

got it!

@chernser
Copy link
Contributor

Good day, @maxjustus !

Thank you for the contribution!

However we have fix for the problem in review #2618

@chernser chernser closed this Oct 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants