Skip to content

Conversation

@AMashenkov
Copy link
Member

No description provided.

AMashenkov added 12 commits May 30, 2022 17:22
# Conflicts:
#	modules/api/src/main/java/org/apache/ignite/sql/ColumnMetadata.java
# Conflicts:
#	modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/AsyncResultSetImpl.java
@AMashenkov AMashenkov requested review from korlov42 and tledkov and removed request for korlov42 June 2, 2022 10:15
@Override
public ResultSetMetadata metadata() {
return List::of;
return ResultSetMetadataImpl.NO_METADATA;
Copy link
Contributor

Choose a reason for hiding this comment

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

let's introduce a constant for DDL meta with a single column of type Bool and name "Applied"

Copy link
Member Author

Choose a reason for hiding this comment

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

DDL has no resultset.
Does it make sense to return any meta?

Copy link
Contributor

Choose a reason for hiding this comment

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

DDL has no resultset

it's true for public API.

Please take a look at org.apache.ignite.internal.sql.engine.exec.ExecutionServiceImpl#executeDdl. There a single column response is returned for the every DDL operation. Next, this cursor will be wrapped with AsyncSqlCursor which enrich the data with results metadata. So, I would expect that AsyncSqlCursor opened for a DDL command will provide a valid meta for the result (it's currently an empty meta which is wrong).

But later, when AsyncSqlCursor will be wrapped with AsyncResultSet, the meta for DDL operation should be omitted.
The reasons why I assume this is the right way is described in this comment

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it, thanks.

Copy link
Member Author

@AMashenkov AMashenkov Jun 3, 2022

Choose a reason for hiding this comment

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

let's introduce a constant for DDL meta.

Seems, we can do the same with DML, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, indeed

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, as I could see, it's already derived from a query plan... So, it's up to you to change this to predefined meta or leave it as is

AMashenkov and others added 6 commits June 3, 2022 17:13
@AMashenkov AMashenkov requested a review from korlov42 June 3, 2022 18:26
@AMashenkov AMashenkov merged commit 7931c09 into main Jun 6, 2022
@AMashenkov AMashenkov deleted the ignite-16962 branch June 6, 2022 15:01
isapego pushed a commit to isapego/ignite-3 that referenced this pull request Dec 26, 2024
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.

3 participants