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.

Sign up for GitHub

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

Jump to bottom

Infer timestamp column when possible #1144

Closed
jiacai2050 opened this issue Aug 9, 2023 · 2 comments
Closed

Infer timestamp column when possible #1144

jiacai2050 opened this issue Aug 9, 2023 · 2 comments
Labels
A-SQL Area: SQL layer contributor friendly Good for contribution feature New feature or request good first issue Good for newcomers

Comments

@jiacai2050
Copy link
Contributor

jiacai2050 commented Aug 9, 2023 •
edited
Loading

Describe This Problem

When create table with sql below

CREATE TABLE `abc` (
    `tag` string TAG,
    `id` int TAG,
    `value` double NOT NULL,
    `t` timestamp NOT NULL,
    )

It will throw

MysqlWorker on_query failed. err:Failed to handle sql:CREATE TABL
E `abc` (                                                                                                                          `tag` string TAG,                                          
    `id` int TAG,                                                                                                                  `value` double NOT NULL,                                   
    `t` timestamp NOT NULL,                                                                                                        ), err:Rpc error, code:500, message:Failed to create plan, query:CREATE TABLE `abc` (                                          `tag` string TAG,                                                                                                          
    `id` int TAG,                                                                                                                  `value` double NOT NULL,                                                                                                       `t` timestamp NOT NULL,                                                                                                    
    ), err:Failed to create plan, err:Table must contain timestamp constraint

Proposal

In theory, if there is only one timestamp column in our SQL, we can refer this without user to set explicitly.

Additional Context

No response
@jiacai2050 jiacai2050 added feature New feature or request good first issue Good for newcomers contributor friendly Good for contribution A-SQL Area: SQL layer labels Aug 9, 2023
@Dennis40816
Copy link
Contributor

Hi, @jiacai2050. Is anyone currently working on this? If not, I'd like to give it a try.

@chunshao90
Copy link
Contributor

Hi, @jiacai2050. Is anyone currently working on this? If not, I'd like to give it a try.

Thanks. Feel free to try it.

Dennis40816 added a commit to Dennis40816/ceresdb that referenced this issue Oct 17, 2023
@Dennis40816
feat(query_frontend): infer timestamp for table creation
daabfae 
This commit addresses issue apache#1144. It aims to automatically infer the
timestamp column during table creation when possible.

Previously, users had to explicitly set a timestamp constraint, adding
unnecessary complexity.

Now, the system will detect if there is only one timestamp column and
set it as the constraint.

- Added `build_and_check_timestamp_key_constraint` to check and infer.
- Modified `create_table` to call the new function.
@Dennis40816 Dennis40816 mentioned this issue Oct 17, 2023
Merged
jiacai2050 pushed a commit that referenced this issue Oct 18, 2023
@Dennis40816
feat: infer timestamp constraint for single-timestamp column (#1266)
fb783f0 
## Rationale

This PR aims to resolve issue #1144 by simplifying the process of table
creation with timestamp constraints. Specifically, the system will
automatically infer and set the timestamp constraint under the following
conditions:

1. No timestamp constraint has been previously set.
2. There exists exactly one column with a timestamp data
type(`DataType::Timestamp`).

By introducing this automatic inference, we reduce the complexity for
users who otherwise would have to manually set this constraint.

## Detailed Changes

- **Automatic Inference of Timestamp Constraint**: The system now
detects if there is only a single timestamp column during table creation
and automatically sets it as the timestamp constraint.
- Added a function `build_and_check_timestamp_key_constraint` for
checking and inference.
  - Modified the `create_table` function to call the new function.

- **Updated Test Expectations for `create_table`**:
- Refactored test cases for `create_table` in `parser.rs` to align with
the latest timestamp inference changes.
- Updated the expected statements to include unique constraints for
timestamp columns.

## Test Plan

Utilized the default CI Action for testing. The result was a
[pass](https://github.com/Dennis40816/ceresdb/actions/runs/6545705109).

## Remarks

- Made modifications to a particular section of the code. Would
appreciate it if experienced team members could review and confirm
whether any changes or corrections are needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-SQL Area: SQL layer contributor friendly Good for contribution feature New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jiacai2050 @chunshao90 @Dennis40816