Rename listTables to listRelations to avoid confusion#19920
Rename listTables to listRelations to avoid confusion#19920
Conversation
The "tables" is often used to refer to tables only. "Relations" better indicates the method includes tables, views and materialized views.
github-actions
bot
added
jdbc
mosabua
left a comment
mosabua
left a comment
There was a problem hiding this comment.
Is there no better name for these objects in a schema? Relations is just very abstract. Objects is maybe a bit to broad since other things like functions can be found in a schema ..
What does the SQL spec call them @martint ?
In either way .. I like the idea to rename this .. but it should be consistent and hence also touch the result returned from the methods
| } | ||
|
|
||
| public static Set<SchemaTableName> listTables(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix) | ||
| public static Set<SchemaTableName> listRelations(Session session, Metadata metadata, AccessControl accessControl, QualifiedTablePrefix prefix) |
There was a problem hiding this comment.
At min you should add javadoc here .. but it also looks weird .. you are calling listRelations .. but getting back a set of SchemaTableName objects? Would you not have to rename SchemaTablenName to SchemaRelation as well then to be consistent?
There was a problem hiding this comment.
In other places we use QualifiedObjectName ..
See my comment here: #19832 (comment) |
|
At my workplace, lot of people refer to tables/views/mv as subjects. listRelations sounds bit of a departure from the nomenclature in rest of the codebase even though I understand the reasoning behind it. |
@martint can you put your thoughts on the replies to your comment on the other PR. The points raised there resonate very much with me. |
|
Looking forward to more review comments. |
Sure. I would suggest we rename the methods to: |
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
|
Closing this pull request, as it has been stale for six weeks. Feel free to re-open at any time. |
|
This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua |
6 participants
The "tables" is often used to refer to tables only. "Relations"
better indicates the method includes tables, views and materialized
views.
Extracted from #19832