Refactorings on the JdbcClient methods#27848
Conversation
github-actions
bot
added
druid
| public void rollbackTemporaryTableCreation(ConnectorSession session, JdbcOutputTableHandle handle) | ||
| { | ||
| stats.getRollbackCreateTable().wrap(() -> delegate().rollbackCreateTable(session, handle)); | ||
| stats.getRollbackCreateTable().wrap(() -> delegate().rollbackTemporaryTableCreation(session, handle)); |
There was a problem hiding this comment.
The stats needs to renamed as well, thoughts?
There was a problem hiding this comment.
Does this need RN ?
There was a problem hiding this comment.
It feels a bit strange to have JdbcClientStats for rolling back temporary tables, but not for the destination tables.
Added rollbackDestinationTableCreation to JdbcStats
findinpath
force-pushed
the
findinpath/jdbc-refactorings
branch
3 times, most recently
from
24f9edf to
948d8a8
Compare
|
Rebased on |
findinpath
force-pushed
the
findinpath/jdbc-refactorings
branch
from
948d8a8 to
e2d9265
Compare
|
@chenjian2664 , @findepi could you look over this PR to close on the discussions that emerged from #27702 ? |
|
This pull request has gone a while without any activity. Ask for help on #core-dev on Trino slack. |
plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java
Outdated
Show resolved
Hide resolved
chenjian2664
left a comment
chenjian2664
left a comment
There was a problem hiding this comment.
Looks good % last commit
findinpath
force-pushed
the
findinpath/jdbc-refactorings
branch
from
e2d9265 to
a9c906f
Compare
…ation` Reflect in the naming of the method the fact that the implementation of the method is focused on rolling back the creation of the temporary table.
…ationTableCreation`
findinpath
force-pushed
the
findinpath/jdbc-refactorings
branch
from
a9c906f to
0421379
Compare
|
FYI |
3 participants
Description
This is a follow-up PR generated by the discussions that happened after merging #27702
cc @findepi @chenjian2664
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: