Conversation
Summary of ChangesHello @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 Highlights
🧠 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
Activity
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.
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??? |
There was a problem hiding this comment.
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. |
There was a problem hiding this comment.
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.
| case TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL: { | ||
| break; | ||
| } |
There was a problem hiding this comment.
| int32_t metaUpdateTableChildTableTagValue(SMeta *pMeta, int64_t version, SVAlterTbReq *pReq) { | ||
| int32_t code = TSDB_CODE_SUCCESS; | ||
| TAOS_RETURN(code); | ||
| } |
There was a problem hiding this comment.
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.
| } else if (vAlterTbReq.action == TSDB_ALTER_TABLE_UPDATE_CHILD_TABLE_TAG_VAL) { | ||
| // TODO: | ||
| } |
| static int32_t doRewriteAlterChildTableTagVal(STranslateContext* pCxt, SQuery* pQuery, bool isVirtual) { | ||
| int32_t code = TSDB_CODE_SUCCESS; | ||
| SAlterTableStmt* pStmt = (SAlterTableStmt*)pQuery->pRoot; | ||
|
|
||
| return code; | ||
| } |
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.