Skip to content

feat: batch tag value update#34564

Draft
localvar wants to merge 6 commits into3.0from
feat/batch-tag-modify
Draft

feat: batch tag value update#34564
localvar wants to merge 6 commits into3.0from
feat/batch-tag-modify

Conversation

@localvar
Copy link
Contributor

Description

Issue(s)

  • Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @localvar, 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 significantly enhances the database's ALTER TABLE capabilities by enabling batch updates of tag values across multiple tables and for child tables, including advanced string manipulation using regular expressions. The changes involve extending the core message types, AST representation, and parsing mechanisms, along with integrating the new operations into the metadata and vnode processing layers to ensure robust and efficient execution of these complex alter statements.

Highlights

  • Batch Tag Value Update Functionality: Introduced new ALTER TABLE actions (TSDB_ALTER_TABLE_UPDATE_MULTI_TABLE_TAG_VAL and TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL) to support updating tag values for multiple tables or child tables in a single operation.
  • New AST Nodes and Parsing Logic: Added QUERY_NODE_UPDATE_TAG_VALUE and QUERY_NODE_ALTER_TABLE_UPDATE_TAG_VAL_CLAUSE to the Abstract Syntax Tree (AST) to represent the new batch tag update statements. The SQL parser (sql.y) was updated to recognize and parse these new syntax forms, including support for REGEXP_REPLACE in tag value expressions.
  • Refactoring and Code Structure Improvements: Refactored the SMultiTagUpateVal struct name to SMultiTagUpdateVal (fixing a typo) and introduced SUpdateTableTagVal to encapsulate table-specific tag update information. Existing functions for encoding, decoding, cloning, and destroying AST nodes and messages were extended or modified to handle the new data structures and operations.
  • Metadata and Vnode Integration: Implemented new metadata functions (metaUpdateTableMultiTableTagValue, metaUpdateTableChildTableTagValue) and updated vnode processing logic (vnodeSvr.c, vnodeStream.c) to correctly handle and propagate these batch tag update requests across the system, ensuring consistency and proper authorization checks.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • include/common/tmsg.h
    • Added new TSDB_ALTER_TABLE_UPDATE_MULTI_TABLE_TAG_VAL and TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL constants.
    • Corrected typo in SMultiTagUpateVal to SMultiTagUpdateVal.
    • Introduced SUpdateTableTagVal struct for batch table tag updates.
    • Added tables field to SVAlterTbReq to hold multiple table update requests.
    • Declared tfreeUpdateTableTagVal for freeing SUpdateTableTagVal structures.
  • include/libs/nodes/cmdnodes.h
    • Defined new SUpdateTagValueNode struct for individual tag value updates, including regex fields.
    • Defined SAlterTableUpdateTagValClause struct to represent a clause for updating tags on a single table.
    • Modified SAlterTableStmt to use a generic pList for various alter types and added pWhere for conditional updates.
    • Removed the SAlterTableMultiStmt struct.
  • source/client/src/clientRawBlockWrite.c
    • Updated references from SMultiTagUpateVal to SMultiTagUpdateVal.
  • source/common/src/msg/tmsg.c
    • Updated references from SMultiTagUpateVal to SMultiTagUpdateVal.
    • Implemented encoding and decoding logic for TSDB_ALTER_TABLE_UPDATE_MULTI_TABLE_TAG_VAL and TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL actions.
    • Added tfreeUpdateTableTagVal function for proper memory management of SUpdateTableTagVal.
  • source/dnode/vnode/src/meta/metaTable.c
    • Declared new metadata functions: metaUpdateTableMultiTableTagValue and metaUpdateTableChildTableTagValue.
    • Updated metaAlterTable to dispatch to these new functions for multi-table and child-table tag updates.
    • Commented out the old TSDB_ALTER_TABLE_UPDATE_TAG_VAL and TSDB_ALTER_TABLE_UPDATE_MULTI_TAG_VAL cases in metaAlterTable.
  • source/dnode/vnode/src/meta/metaTable2.c
    • Refactored the core logic for updating multiple tag values into a new static function metaUpdateTableMultiTagValueImpl.
    • Implemented metaUpdateTableMultiTableTagValue to iterate and apply tag updates across multiple tables.
    • Implemented metaUpdateTableChildTableTagValue (currently a placeholder).
    • Updated references from SMultiTagUpateVal to SMultiTagUpdateVal.
  • source/dnode/vnode/src/vnd/vnodeStream.c
    • Added a TODO comment to scanAlterTableNew regarding the new alter table actions.
  • source/dnode/vnode/src/vnd/vnodeSvr.c
    • Modified alterTagForTmq to accept table name and tags array as parameters, making it more generic.
    • Updated vnodeProcessAlterTbReq to iterate through tables and call alterTagForTmq for TSDB_ALTER_TABLE_UPDATE_MULTI_TABLE_TAG_VAL.
  • source/libs/nodes/src/nodesCloneFuncs.c
    • Added updateTagValueNodeCopy for cloning SUpdateTagValueNode instances.
    • Added alterTableUpdateTagValClauseCopy for cloning SAlterTableUpdateTagValClause instances.
    • Updated nodesCloneNode to handle the new node types.
  • source/libs/nodes/src/nodesCodeFuncs.c
    • Added string representations for QUERY_NODE_UPDATE_TAG_VALUE and QUERY_NODE_ALTER_TABLE_UPDATE_TAG_VAL_CLAUSE.
    • Implemented updateTagValueNodeToJson and jsonToUpdateTagValueNode for JSON serialization/deserialization of tag update nodes.
    • Implemented alterTableUpdateTagValClauseToJson and jsonToAlterTableUpdateTagValClause for JSON serialization/deserialization of alter clause nodes.
  • source/libs/nodes/src/nodesUtilFuncs.c
    • Added node creation logic for QUERY_NODE_UPDATE_TAG_VALUE and QUERY_NODE_ALTER_TABLE_UPDATE_TAG_VAL_CLAUSE.
    • Updated nodesDestroyNode to correctly free memory for SUpdateTagValueNode (including regex fields) and SAlterTableUpdateTagValClause (destroying its tag list).
    • Modified nodesDestroyNode for SAlterTableStmt to destroy pWhere and pList instead of pNodeListTagValue.
  • source/libs/parser/inc/parAst.h
    • Renamed createAlterSingleTagColumnNode to createAlterTagValueNode.
    • Added createAlterTagValueNodeWithExpression for tag updates using regular expressions.
    • Added createAlterTableUpdateTagValClause for creating a clause for a single table's tag updates.
    • Added createAlterMultiTableUpdateTagValStmt for creating statements to update tags across multiple tables.
    • Added createAlterChildTableUpdateTagValStmt for creating statements to update tags on child tables with a WHERE clause.
  • source/libs/parser/inc/sql.y
    • Modified grammar to define column_tag_value and column_tag_value_list for tag updates.
    • Introduced alter_single_table_clause and alter_multi_table_clause for batch table tag updates.
    • Added grammar rules for alter_table_clause to support ALTER TABLE ... SET TAG ... for multiple tables.
    • Introduced column_tag_expr_value to support REGEXP_REPLACE in tag value updates.
    • Added grammar rules for alter_table_clause to support OF super_table SET TAG ... WHERE ... for child table tag updates.
  • source/libs/parser/src/parAstCreater.c
    • Implemented createAlterTagValueNode to create SUpdateTagValueNode for simple tag value assignments.
    • Implemented createAlterTagValueNodeWithExpression to create SUpdateTagValueNode for regex-based tag value replacements.
    • Implemented createAlterTableUpdateTagValClause to create SAlterTableUpdateTagValClause.
    • Implemented createAlterMultiTableUpdateTagValStmt to create SAlterTableStmt for multi-table tag updates.
    • Implemented createAlterChildTableUpdateTagValStmt to create SAlterTableStmt for child table tag updates.
    • Removed old functions createAlterSingleTagColumnNode and createAlterTableSetTag.
  • source/libs/parser/src/parAstParser.c
    • Updated collectMetaKeyFromAlterTable to handle metadata collection for TSDB_ALTER_TABLE_UPDATE_MULTI_TABLE_TAG_VAL and TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL.
    • Updated collectMetaKeyFromAlterVtable to include TODOs for new alter types.
  • source/libs/parser/src/parTokenizer.c
    • Added REGEXP_REPLACE as a new keyword.
  • source/libs/parser/src/parTranslater.c
    • Updated checkAlterSuperTable to prevent new tag update operations on supertables.
    • Refactored buildUpdateTagValReqImpl2 to accept tagStr directly.
    • Commented out the old buildUpdateTagValReqImpl and buildUpdateMultiTagValReq functions.
    • Introduced SVgroupAlterTableReq struct and associated destroyVgroupAlterTableReq, serializeVgroupAlterTableReq, and serializeVgroupsAlterTableReq functions for handling vgroup-specific alter requests.
    • Implemented doRewriteAlterMultiTableTagVal to process and serialize multi-table tag update requests.
    • Implemented doRewriteAlterChildTableTagVal (currently a placeholder).
    • Refactored rewriteAlterTable to dispatch to doRewriteAlterMultiTableTagVal, doRewriteAlterChildTableTagVal, or doRewriteAlterTable based on the alter type.
Activity
  • The pull request introduces new message types and AST nodes to support batch tag value updates.
  • Significant changes were made to the parser to handle new SQL syntax for these operations, including regular expression-based replacements.
  • The metadata and vnode layers were updated to correctly process and manage the new alter table requests.
  • Several existing data structures and functions were refactored or extended to accommodate the new features, including fixing a typo in a struct name.
  • Some sections include 'TODO' comments, indicating potential future work or areas for further development.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces a feature for batch updating tag values, including support for multiple tables and regular expression-based updates. While the security review is still in its initial SAST analysis phase, the code review has identified critical memory management issues that could lead to leaks or crashes. Additionally, there are several pieces of incomplete or commented-out code (// TODO:, #if 0, // useless???) that require attention to finalize the feature and improve maintainability.

#define TSDB_ALTER_TABLE_DROP_TAG 2
#define TSDB_ALTER_TABLE_UPDATE_TAG_NAME 3
#define TSDB_ALTER_TABLE_UPDATE_TAG_VAL 4
#define TSDB_ALTER_TABLE_UPDATE_TAG_VAL 4 // useless???
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The comment // useless??? has been added. A similar comment exists on line 246. Such comments indicate uncertainty and can lead to confusion. If these macros are no longer used, they should be removed along with their usages. If they are still needed for backward compatibility or other reasons, the comments should be removed.

TAOS_CHECK_EXIT(tEncodeCStr(pEncoder, pReq->colName));
TAOS_CHECK_EXIT(tEncodeCStr(pEncoder, pReq->colNewName));
break;
#if 0 // should be useless, but keep the code for now.
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This block of code is disabled with #if 0 and a comment "should be useless, but keep the code for now.". If this code is truly obsolete, it should be removed to clean up the codebase. Keeping dead code can be confusing for future maintenance. I've noticed similar blocks for related functionality in other files as well.

Comment on lines +15092 to +15094
case TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL: {
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The case for TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL is empty. This indicates an incomplete feature. Please add the implementation or a comment explaining why it's empty. I've noticed several other stubs and TODOs related to this feature across the codebase.

Comment on lines +1743 to +1746
int32_t metaUpdateTableChildTableTagValue(SMeta *pMeta, int64_t version, SVAlterTbReq *pReq) {
int32_t code = TSDB_CODE_SUCCESS;
TAOS_RETURN(code);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The function metaUpdateTableChildTableTagValue is a stub and doesn't have an implementation. This indicates an incomplete feature. Please implement it or add a comment explaining why it's empty if it's intended. I've noticed several other stubs and TODOs related to TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL across the codebase.

Comment on lines +1740 to 1742
} else if (vAlterTbReq.action == TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL) {
// TODO:
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This else if block for TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL contains only a // TODO: comment, indicating an incomplete feature. This should be implemented to complete the functionality.

Comment on lines +24071 to +24076
static int32_t doRewriteAlterChildTableTagVal(STranslateContext* pCxt, SQuery* pQuery, bool isVirtual) {
int32_t code = TSDB_CODE_SUCCESS;
SAlterTableStmt* pStmt = (SAlterTableStmt*)pQuery->pRoot;

return code;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The function doRewriteAlterChildTableTagVal is a stub and returns TSDB_CODE_SUCCESS without any implementation. This indicates an incomplete feature that should be implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant