fix: Increse size of type and value in task input/output and size of data - #92
fix: Increse size of type and value in task input/output and size of data#92sitaowang1998 wants to merge 2 commits into
Conversation
WalkthroughThis pull request increases the maximum allowed lengths for certain columns in several database tables. Both the C++ header file used for table definitions and the SQL initialisation script have been updated. The changes expand the size of the Changes
Assessment against linked issues
Possibly related PRs
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/spider/storage/mysql/mysql_stmt.hpp (1)
157-174: Consider documenting migration path for existing databases.While the schema changes are consistent across files, there's no information about how existing databases should be migrated to the new schema. Consider adding documentation or a migration script for existing deployments.
Would you like me to generate a sample SQL migration script to update existing database schemas to match these changes?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/spider/storage/mysql/mysql_stmt.hpp(4 hunks)tools/scripts/storage/init_db.sql(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint
- GitHub Check: non-storage-unit-tests (ubuntu-22.04)
- GitHub Check: non-storage-unit-tests (ubuntu-24.04)
🔇 Additional comments (11)
tools/scripts/storage/init_db.sql (5)
61-61: Good adjustment to the VARBINARY size for the data table value field.The increase from 256 to 999 bytes for the
valuecolumn will allow for storing larger data payloads, addressing the issue mentioned in PR objectives about insufficient sizes for real-world production scenarios.
70-71: Increased sizes for task_outputs columns is appropriate.The change from VARCHAR(64) to VARCHAR(999) for
typeand VARBINARY(64) to VARBINARY(999) forvaluewill accommodate more complex type descriptors and larger output values, aligning with the PR objectives.
81-84: Appropriate size increase for task_inputs columns.The larger sizes for
type(VARCHAR(999)) andvalue(VARBINARY(999)) columns will ensure that task inputs can handle more complex data structures and larger payloads as required in production environments.
138-138: Good adjustment for client_kv_data value field size.Increasing the
valuecolumn from VARBINARY(128) to VARBINARY(999) will allow clients to store larger key-value data as needed in production environments.
145-145: Appropriate size increase for task_kv_data value field.The increase of the
valuecolumn from VARBINARY(128) to VARBINARY(999) is consistent with the other changes and addresses the need for larger data storage in task key-value pairs.src/spider/storage/mysql/mysql_stmt.hpp (6)
70-73: Good alignment with SQL schema changes for task_inputs.The changes to increase the
typeandvaluecolumns to VARCHAR(999) and VARBINARY(999) respectively match the changes in the init_db.sql file, ensuring consistency between code and database schema.
84-85: Properly synchronized task_outputs column size changes.The increases to VARCHAR(999) for
typeand VARBINARY(999) forvaluematch the corresponding changes in the init_db.sql file, maintaining schema consistency.
111-111: Consistent data value column size increase.The change from VARBINARY(256) to VARBINARY(999) for the
valuecolumn in the data table matches the SQL schema change, ensuring consistency.
144-144: Consistent client_kv_data value size adjustment.The increase to VARBINARY(999) for the
valuecolumn properly aligns with the corresponding change in the init_db.sql file.
151-151: Correctly synchronized task_kv_data value column size.The increase to VARBINARY(999) for the
valuecolumn matches the change in the init_db.sql file, maintaining schema consistency.
111-111:Details
❓ Verification inconclusive
Consider the specific choice of 999 for column sizes.
The consistent choice of 999 bytes (rather than 1000, 1024, etc.) for all affected columns is interesting. Is there a specific reason for this value? Consider whether this size will be sufficient for future needs or if a more generous size would prevent another increase later.
Also applies to: 70-73, 84-85, 144-144, 151-151
🌐 Web query:
What is the practical maximum size for MySQL VARBINARY columns?💡 Result:
The practical maximum size for a
VARBINARYcolumn in MySQL is 65,535 bytes, but this is subject to the following constraints:
Row Size Limit:
The total row size (including all columns) cannot exceed MySQL’s maximum row size of 65,535 bytes[3][6][9]. For example:
- A table with a single
VARBINARY(65535)column is valid.- If other columns exist, their combined size reduces the maximum allowed
VARBINARYlength[3][9].Storage Engine Impact:
InnoDB imposes additional constraints. For tables using the default 16KB page size, rows must fit within ~8KB for on-page storage. LargerVARBINARYvalues may trigger off-page storage (via overflow pages), but this depends on theROW_FORMAT(e.g.,DYNAMICallows efficient off-page storage for large values)[8].Alternatives for Larger Data:
For binary data exceeding 65,535 bytes, useBLOBtypes:
BLOB: Up to 65,535 bytes (inline storage, contributes to row size).MEDIUMBLOB: Up to 16MB.LONGBLOB: Up to 4GB[2][6][9].Example Scenarios
Use Case Recommended Type Maximum Size Notes Small binary data (e.g., flags, short hashes) VARBINARY(255)≤65,535 bytes Fits entirely within row. Large files (e.g., images, PDFs) LONGBLOB4GB Stored off-page with minimal row overhead[8]. To define a
VARBINARYcolumn:CREATE TABLE example ( id INT PRIMARY KEY, data VARBINARY(10000) -- Adjust size based on row limits );Always validate the maximum usable length using
EXECUTE ... SELECTor schema checks, especially in multi-column tables[3][6].Citations:
- 1: https://www.dofactory.com/sql/varbinarymax
- 2: https://www.w3schools.com/mysql/mysql_datatypes.asp
- 3: https://dev.mysql.com/doc/mysql-reslimits-excerpt/5.7/en/column-count-limit.html
- 4: https://www.mysqltutorial.org/mysql-basics/mysql-varbinary/
- 5: https://nelkodev.com/en/blog/efficient-data-handling-with-varbinary-in-mysql/
- 6: https://docs.oracle.com/cd/E19078-01/mysql/mysql-refman-5.1/data-types.html
- 7: https://wiki.ispirer.com/sqlways/mysql/data-types/varbinary
- 8: https://dev.mysql.com/doc/refman/9.0/en/innodb-row-format.html
- 9: https://dev.mysql.com/doc/refman/8.1/en/string-type-syntax.html
- 10: https://learn.microsoft.com/en-us/sql/t-sql/data-types/binary-and-varbinary-transact-sql?view=sql-server-ver16
Review Directive: Verify and Document the Rationale for VARBINARY Column Size
The current implementation in
src/spider/storage/mysql/mysql_stmt.hppdefines thevaluecolumn asVARBINARY(999), which is substantially lower than the practical maximum of 65,535 bytes for a VARBINARY column (subject to row size and storage engine constraints). Please confirm whether this 999-byte limitation is intentional—perhaps based on expected data sizes or performance considerations—and consider if future requirements might benefit from a more generous allowance. If the choice is deliberate, it would be helpful to document the rationale inline. This consideration also applies to the similar column definitions at lines 70–73, 84–85, 144, and 151.
1 participant
Description
The size of type and value task input/output is set to 64 and 256, while size of data is set to 256. These numbers are too small for real-world production.
This pr also increases the above mentioned sizes. Fixes #68.
Checklist
breaking change.
Validation performed
Summary by CodeRabbit