Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port of #71984- Fix for arrayWithConstant size estimation using row's element size #631

Conversation

onkar
Copy link

@onkar onkar commented Feb 15, 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):

Port of ClickHouse#71894 from ClickHouse upstream. While upgrading to Altinity's version 24.8.11.51285, we started seeing this issue. After doing a bit of digging, it appears that ClickHouse#71894 is missing from the master branch.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/


Modify your CI run:

NOTE: If your merge the PR with modified CI you MUST KNOW what you are doing
NOTE: Checked options will be applied if set before CI RunConfig/PrepareRunConfig step

Include tests (required builds will be added automatically):

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Unit tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with Analyzer
  • Add your option here

Exclude tests:

  • Fast test
  • Integration Tests
  • Stateless tests
  • Stateful tests
  • Performance tests
  • All with ASAN
  • All with TSAN
  • All with MSAN
  • All with UBSAN
  • All with Coverage
  • All with Aarch64
  • Add your option here

Extra options:

  • do not test (only style check)
  • disable merge-commit (no merge from master before tests)
  • disable CI cache (job reuse)

Only specified batches in multi-batch jobs:

  • 1
  • 2
  • 3
  • 4

@onkar
Copy link
Author

onkar commented Feb 16, 2025

@MyroTk apologies for tagging you but can you please take a look? Thanks!

@Enmk
Copy link
Member

Enmk commented Feb 19, 2025

Hi @onkar ! Thank you for contribution! We have a bit more complex workflow here, comparing to upstream, and basically we don't merge anything into master.
So the question is at what version do you want this to be released? Options are:

  • upcoming 24.8 -- then please update a PR to point onto customizations/24.8.14 (I can change the PR's base for you)
  • antalya (kind of a experimental collection of features, which follows upstream's releases more closely than Altinity Stable Builds) - then please update PR to point onto an antalya branch.

Or I can do it for you

@Enmk Enmk changed the base branch from master to antalya February 19, 2025 21:19
@Enmk Enmk changed the base branch from antalya to master February 19, 2025 21:19
@onkar
Copy link
Author

onkar commented Feb 19, 2025

Hi @onkar ! Thank you for contribution! We have a bit more complex workflow here, comparing to upstream, and basically we don't merge anything into master. So the question is at what version do you want this to be released? Options are:

  • upcoming 24.8 -- then please update a PR to point onto customizations/24.8.14 (I can change the PR's base for you)
  • antalya (kind of a experimental collection of features, which follows upstream's releases more closely than Altinity Stable Builds) - then please update PR to point onto an antalya branch.

Or I can do it for you

Hello @Enmk, I would like the fix released in the upcoming 24.8 version. We saw this in internal testing and it would be nice to resolve that issue after the upgrade. Please change the PR's base for me. Thank you for taking a look!

@onkar onkar changed the base branch from master to customizations/24.8.14 February 19, 2025 22:15
@onkar
Copy link
Author

onkar commented Feb 20, 2025

@Enmk, I am running into unrelated merge conflicts after changing the base branch to customizations/24.8.14. Can you please help with the next steps? I tried opening #642 directly on the branch but that did not help.

@Enmk
Copy link
Member

Enmk commented Feb 20, 2025

@Enmk, I am running into unrelated merge conflicts after changing the base branch to customizations/24.8.14. Can you please help with the next steps? I tried opening #642 directly on the branch but that did not help.

Please figure out conflict in tests/queries/0_stateless/03216_arrayWithConstant_limits.sql, everything else you can resolve using Altinity's files.

@Enmk
Copy link
Member

Enmk commented Feb 21, 2025

closed in favor of #650

@Enmk Enmk closed this Feb 21, 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.

5 participants