-
Notifications
You must be signed in to change notification settings - Fork 5k
feat: [TS-6665] Support 65535 cols. #33743
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
base: 3.0
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @Simon9997, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement by increasing the maximum number of columns supported within the database system. This change directly addresses the need for greater flexibility in schema design, allowing for tables with up to 65535 columns, thereby accommodating more complex and extensive datasets. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request increases the maximum number of columns to 65535. While the change is small, it has critical side effects. The data type for column IDs (col_id_t) is an int16_t, which will overflow with more than 32,767 columns, leading to severe bugs. This must be changed to int32_t throughout the codebase. Additionally, this change significantly increases the size of several memory buffers (e.g., up to 16MB), which could cause performance issues or out-of-memory errors. These impacts need to be carefully evaluated, and the empty pull request description should be updated to reflect the scope and consequences of this feature.
| #define TSDB_VGROUP_ID_LEN 11 | ||
|
|
||
| #define TSDB_MAX_COLUMNS 4096 | ||
| #define TSDB_MAX_COLUMNS 65535 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Increasing TSDB_MAX_COLUMNS to 65535 is not safe due to a data type overflow. The type col_id_t, used for column IDs, is defined as int16_t in include/common/ttypes.h, which can only hold values up to 32,767. Using column IDs larger than this will cause an integer overflow, leading to unpredictable behavior and likely data corruption.
Given that special negative values like TSDB_TBNAME_COLUMN_INDEX (-1) are used for column identifiers, col_id_t and all related fields (e.g., SColumn.colId) must be changed to a larger signed type, such as int32_t, to safely accommodate up to 65,535 columns. This is a critical change that needs to be applied consistently across the entire codebase wherever column IDs are handled.
| #define TSDB_VGROUP_ID_LEN 11 | ||
|
|
||
| #define TSDB_MAX_COLUMNS 4096 | ||
| #define TSDB_MAX_COLUMNS 65535 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change significantly increases the memory footprint of several components. Buffers sized with TSDB_MAX_COLUMNS will become much larger. For example:
TSDB_MAX_SAVED_SQL_LEN(line 302) will grow from ~256 KB to ~4 MB.TSDB_SHOW_SCHEMA_JSON_LEN(line 341) will grow from 1 MB to ~16 MB.
These large allocations, especially if on the stack, could lead to stack overflows or out-of-memory errors in environments with limited resources. Please confirm that this increased memory usage is acceptable and has been tested. The impact of these changes should be documented in the pull request description.
Description
Please briefly describe the code changes in this pull request.
Jira: https://jira.taosdata.com:18080/browse/TD-
Checklist
Please check the items in the checklist if applicable.