Skip to content

Better management of DateTime partitionning #3786 #3800

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

Merged
merged 7 commits into from
Dec 13, 2018

Conversation

edonin
Copy link
Contributor

@edonin edonin commented Dec 10, 2018

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Has discussed in the issue #3786, I added two columns called "min_time" and "max_time" in the system.parts table. They contain the min datetime and max datetime of the partition if the merge tree is using one and only datetime in the sorting index. Otherwise, these columns are equals to 0.

@alexey-milovidov
Copy link
Member

Missing functional test. Look at dbms/tests/queries directory.

@alexey-milovidov
Copy link
Member

I think, the user will expect, that min_date, max_date columns will be also filled with non-zero value, if there is min_time, max_time.

Copy link
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

.

@edonin
Copy link
Contributor Author

edonin commented Dec 11, 2018

For the tests, I will take a look.

@edonin
Copy link
Contributor Author

edonin commented Dec 11, 2018

For the other issue, I have a doubt.

For my understanding, DateTime is timezone agnostic (it could be seen to any timezone, and still refer to the same instant). But Date is timezone dependant. So for converting a DateTime to Date, you need to specify a timezone. So here, which timezone could we use for converting the DateTime to Date ? The system timezone ? The user timezone ?

Thanks,

@alexey-milovidov
Copy link
Member

For my understanding, DateTime is timezone agnostic (it could be seen to any timezone, and still refer to the same instant). But Date is timezone dependant. So for converting a DateTime to Date, you need to specify a timezone. So here, which timezone could we use for converting the DateTime to Date ? The system timezone ? The user timezone ?

You're right, let leave it as is.

@edonin
Copy link
Contributor Author

edonin commented Dec 12, 2018

I added some tests.

Nevertheless, the result is a bit different of what i described in the issue. The column min_time and max_time are filled if the merge tree partitionning index contains a DateTime. It does not care about the merge tree sorting index.

Since these two columns is defined for each partition, it seems more logical that they depend of the partitionning index.

issue with reference file for tests
issue with reference file for test
it is due to partition naming that seems to have changed recently.
@alexey-milovidov alexey-milovidov merged commit 7898cae into ClickHouse:master Dec 13, 2018
@alexey-milovidov
Copy link
Member

Ok.

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.

3 participants