fix:retry with table meta change#33977
Conversation
Summary of ChangesHello @Pengrongkun, 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 enhances the robustness of 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. 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 fix to handle table metadata changes during INSERT operations by triggering a retry. The implementation is sound, but I've suggested a small improvement to make the retry condition more specific to avoid unnecessary retries on genuine syntax errors. Overall, a good enhancement for robustness.
| if (!pCxt->forceUpdate) { | ||
| code = TSDB_CODE_TDB_INVALID_TABLE_SCHEMA_VER; | ||
| parserWarn("QID:0x%" PRIx64 ", column number is smaller than %d, need retry", pCxt->pComCxt->requestId, | ||
| pCols->numOfBound); | ||
| } else { | ||
| code = buildSyntaxErrMsg(&pCxt->msg, ", expected", pToken->z); | ||
| } |
There was a problem hiding this comment.
The current logic for retrying on schema changes is a bit too broad. It triggers a retry for any token that is not a comma, which includes genuine syntax errors like a missing comma between values (e.g., VALUES(1 2)). This leads to an unnecessary retry cycle for what is a clear syntax error.
To make the retry mechanism more precise, we should only trigger it when we encounter a closing parenthesis ) prematurely, as this is a clear signal that the client is providing fewer columns than the server expects due to a stale schema. Other cases should be treated as syntax errors immediately.
if (pToken->type == TK_NK_RP && !pCxt->forceUpdate) {
code = TSDB_CODE_TDB_INVALID_TABLE_SCHEMA_VER;
parserWarn("QID:0x%" PRIx64 ", column number is smaller than %d, need retry", pCxt->pComCxt->requestId,
pCols->numOfBound);
} else {
code = buildSyntaxErrMsg(&pCxt->msg, ", expected", pToken->z);
}* 3.0: (1127 commits) fix: confilcts enh(grant): support data source Pulsar (#33978) feat: support TOTP code login and password expired tip (#33969) refactor: support Python3.12 (#33985) fix: audit req, select and insert support (#33960) fix: [6589381451] Add virtual table datatype check when query. (#33973) fix: [6589381451] Add virtual table datatype check when query. (#33972) fix: fix wal unittest (#33970) fix: retry with table meta change (#33977) fix: ci case docs: update 02-concept.md (#33981) fix: build error fix(TS-7676): sub query with window main test (#33790) docs: update nodejs docs (#33807) enh: window query without agg function support (#33759) fix lint docs: update cloud quick start Update new-framework-test.yml (#33966) Update new-framework-test.yml (#33965) feat: TS-7270 add doc (#33955) ...
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.