Skip to content

Conversation

@Young-Leo
Copy link
Contributor

No description provided.

Copy link
Contributor

@JackieTien97 JackieTien97 left a comment

Choose a reason for hiding this comment

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

IMO, making Node implements Accountable interface may be the best way.

}

private void addNodeSize(Node node) {
totalMemorySize += RamUsageEstimator.shallowSizeOfInstance(node.getClass());
Copy link
Contributor

Choose a reason for hiding this comment

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

Node's InstanceSize should be a constant, no need to calculate each time.

import java.util.Optional;

/** Utility class for estimating memory usage of AST nodes. */
public final class AstMemoryEstimator {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's hard to maintain this class, if any Node has some changes or new Node being added.

…are-ast

# Conflicts:
#	iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/ast/LoadTsFile.java
Copy link
Contributor

@JackieTien97 JackieTien97 left a comment

Choose a reason for hiding this comment

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

Remember to review the codes by yourself if the codes are mostly generated by LLM.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds memory estimation capabilities for SQL AST (Abstract Syntax Tree) nodes by implementing the Accountable interface across the codebase. The feature enables tracking memory usage of parsed SQL statements through the ramBytesUsed() method, which is useful for memory management and resource planning.

Key Changes:

  • Implements Accountable interface on the base Node and JoinCriteria classes
  • Adds ramBytesUsed() method implementations to 100+ AST node classes
  • Introduces comprehensive test suite with 80+ test cases covering various SQL constructs
  • Adds getLocationInternal() accessor method to Node class for memory estimation

Reviewed changes

Copilot reviewed 207 out of 207 changed files in this pull request and generated no comments.

Show a summary per file
File Description
AstMemoryEstimatorTest.java New comprehensive test suite with 80+ tests covering literals, expressions, queries, and complex SQL statements
Node.java Base class updated to implement Accountable and add getLocationInternal() method
JoinCriteria.java Updated to implement Accountable interface
QualifiedName.java Implements Accountable with memory estimation logic for qualified names
ShowDevicesResult.java Adds Accountable implementation for schema result objects
100+ AST classes Each implements ramBytesUsed() using consistent pattern: INSTANCE_SIZE + field sizes
AbstractTraverseDevice.java Adds getter method for attributeColumns field

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@JackieTien97 JackieTien97 merged commit 8a5e1e3 into apache:master Dec 24, 2025
48 of 58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants