Skip to content

Conversation

@chenjian2664
Copy link
Contributor

Description

Additional context and related issues

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Aug 17, 2025
@github-actions github-actions bot added the blackhole Blackhole connector label Aug 17, 2025
}

@Override
public Iterator<RelationColumnsMetadata> streamRelationColumns(ConnectorSession session, Optional<String> schemaName, UnaryOperator<Set<SchemaTableName>> relationFilter)
Copy link
Member

Choose a reason for hiding this comment

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

This line is too long. Please put parameters on separate lines. Also, synchronized looks missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why need synchronized here?

Copy link
Member

Choose a reason for hiding this comment

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

Not specific here. For instance, BlackHoleMetadata.renameTable method contains tables.remove & tables.put calls. Streaming columns may miss tables in the middle of renaming. No need to fix as it's pre-existing though.

@chenjian2664 chenjian2664 force-pushed the blockhole_stream_columns branch from 903e34f to 87dcd36 Compare August 17, 2025 10:04
Copy link
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

Looks good except for a view comment.

@chenjian2664 chenjian2664 force-pushed the blockhole_stream_columns branch from 87dcd36 to ee2216b Compare August 18, 2025 12:19
@ebyhr ebyhr force-pushed the blockhole_stream_columns branch from ee2216b to eeb7867 Compare August 18, 2025 14:20
@ebyhr ebyhr merged commit 7de791c into trinodb:master Aug 18, 2025
52 checks passed
@github-actions github-actions bot added this to the 477 milestone Aug 18, 2025
@chenjian2664
Copy link
Contributor Author

Thank you @ebyhr for clean ups of the code

@chenjian2664 chenjian2664 deleted the blockhole_stream_columns branch August 19, 2025 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blackhole Blackhole connector cla-signed

Development

Successfully merging this pull request may close these issues.

2 participants