Skip to content

Conversation

@hjxilinx
Copy link
Contributor

Description

feat(query): update the perf_queries table schema.

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 @hjxilinx, 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 significant refinements to the perf_queries table schema and its associated data handling. It enhances the granularity of sub-query tracking by adding a virtual group ID and improves the representation of query identifiers and execution times. These changes ensure more detailed and accurate performance monitoring data, facilitating better analysis of query execution.

Highlights

  • Schema Update for SQuerySubDesc: The SQuerySubDesc structure has been extended to include a new vgId (virtual group ID) field, enhancing the ability to track sub-query execution contexts.
  • perf_queries Table Schema Refinement: The perf_queries system table schema has been updated. The query_id field is now stored as a VARCHAR (hex string representation) instead of UBIGINT, and exec_usec has been renamed to exec_sec and its type changed from BIGINT (microseconds) to DOUBLE (seconds) for more precise execution time reporting.
  • Serialization and Deserialization Updates: The message serialization and deserialization logic for SQuerySubDesc has been adjusted to correctly handle the newly added vgId field.
  • Data Packing Logic Adjustment: The data packing mechanism for the perf_queries table has been modified to convert queryId to its hexadecimal string representation and useconds to seconds (double) before storage, aligning with the updated schema.
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 updates the perf_queries table schema to provide more detailed query information, and adds a vgId to SQuerySubDesc to support this. The changes are consistent across struct definitions, serialization/deserialization, and data population logic. The query_id is now a VARCHAR to store hex-formatted IDs, and execution time is stored as a DOUBLE in seconds, which are reasonable changes. I've identified one area for improvement regarding code maintainability in the serialization logic. Overall, the changes are well-implemented and achieve the intended goal.

Comment on lines 297 to +298
TAOS_CHECK_RETURN(tEncodeCStr(pEncoder, sDesc->status));
TAOS_CHECK_RETURN(tEncodeI32(pEncoder, sDesc->vgId));
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 serialization order of fields in SQuerySubDesc (status then vgId) does not match the order in the struct definition in include/common/tmsg.h (vgId then status). While this may not cause a bug currently because the deserialization logic is symmetric, it makes the code harder to read and maintain. It's best practice to keep the serialization, deserialization, and struct definition orders consistent to avoid potential issues in the future. Please update both the serialization and deserialization logic to match the struct definition.

          TAOS_CHECK_RETURN(tEncodeI32(pEncoder, sDesc->vgId));
          TAOS_CHECK_RETURN(tEncodeCStr(pEncoder, sDesc->status));

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.

2 participants