mchades commented on code in PR #10834:
URL: https://github.com/apache/gravitino/pull/10834#discussion_r3125200240
##########
core/src/main/java/org/apache/gravitino/hook/TableHookDispatcher.java:
##########
@@ -79,13 +82,17 @@ public Table createTable(
ident, columns, comment, properties, partitions, distribution,
sortOrders, indexes);
// Set the creator as the owner of the table.
- OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
- if (ownerManager != null) {
- ownerManager.setOwner(
- ident.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.TABLE),
- PrincipalUtils.getCurrentUserName(),
- Owner.Type.USER);
+ try {
+ OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
+ if (ownerManager != null) {
+ ownerManager.setOwner(
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.TABLE),
Review Comment:
With the new dispatcher chain order (`HookDispatcher → EventDispatcher →
NormalizeDispatcher → Manager`), `ident` here is the raw caller input — it has
not yet passed through `NormalizeDispatcher`. For TABLE scope, both the leaf
name and the schema name inside `ident.namespace()` can be normalized, but the
entity is stored under the normalized names. This `setOwner` call may therefore
silently fail (exception swallowed by the `catch` block).
##########
core/src/main/java/org/apache/gravitino/hook/FilesetHookDispatcher.java:
##########
@@ -80,13 +83,17 @@ public Fileset createMultipleLocationFileset(
ident, comment, type, storageLocations, properties);
// Set the creator as the owner of the fileset.
- OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
- if (ownerManager != null) {
- ownerManager.setOwner(
- ident.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.FILESET),
- PrincipalUtils.getCurrentUserName(),
- Owner.Type.USER);
+ try {
+ OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
+ if (ownerManager != null) {
+ ownerManager.setOwner(
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.FILESET),
Review Comment:
Same normalization issue as in `TableHookDispatcher#createTable`.
##########
core/src/main/java/org/apache/gravitino/hook/SchemaHookDispatcher.java:
##########
@@ -60,13 +63,17 @@ public Schema createSchema(NameIdentifier ident, String
comment, Map<String, Str
Schema schema = dispatcher.createSchema(ident, comment, properties);
// Set the creator as the owner of the schema.
- OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
- if (ownerManager != null) {
- ownerManager.setOwner(
- ident.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.SCHEMA),
- PrincipalUtils.getCurrentUserName(),
- Owner.Type.USER);
+ try {
+ OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
+ if (ownerManager != null) {
+ ownerManager.setOwner(
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.SCHEMA),
Review Comment:
Same normalization issue as in `TableHookDispatcher#createTable`. For SCHEMA
scope, only the leaf (schema) name is affected — the namespace
`metalake.catalog` is not normalized by `NormalizeDispatcher`.
##########
core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java:
##########
@@ -69,13 +72,17 @@ public Model registerModel(NameIdentifier ident, String
comment, Map<String, Str
Model model = dispatcher.registerModel(ident, comment, properties);
// Set the creator as owner of the model.
- OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
- if (ownerManager != null) {
- ownerManager.setOwner(
- ident.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.MODEL),
- PrincipalUtils.getCurrentUserName(),
- Owner.Type.USER);
+ try {
+ OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
+ if (ownerManager != null) {
+ ownerManager.setOwner(
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.MODEL),
Review Comment:
Same normalization issue as in `TableHookDispatcher#createTable`.
##########
core/src/main/java/org/apache/gravitino/hook/TopicHookDispatcher.java:
##########
@@ -66,13 +69,17 @@ public Topic createTopic(
Topic topic = dispatcher.createTopic(ident, comment, dataLayout,
properties);
// Set the creator as the owner of the topic.
- OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
- if (ownerManager != null) {
- ownerManager.setOwner(
- ident.namespace().level(0),
- NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.TOPIC),
- PrincipalUtils.getCurrentUserName(),
- Owner.Type.USER);
+ try {
+ OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
+ if (ownerManager != null) {
+ ownerManager.setOwner(
+ ident.namespace().level(0),
+ NameIdentifierUtil.toMetadataObject(ident,
Entity.EntityType.TOPIC),
Review Comment:
Same normalization issue as in `TableHookDispatcher#createTable`.
##########
core/src/main/java/org/apache/gravitino/hook/ModelHookDispatcher.java:
##########
@@ -157,13 +164,17 @@ public Model registerModel(
Model model = dispatcher.registerModel(ident, uris, aliases, comment,
properties);
// Set the creator as owner of the model.
- OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
- if (ownerManager != null) {
- ownerManager.setOwner(
- ident.name(),
- NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.MODEL),
- PrincipalUtils.getCurrentUserName(),
- Owner.Type.USER);
+ try {
+ OwnerDispatcher ownerManager =
GravitinoEnv.getInstance().ownerDispatcher();
+ if (ownerManager != null) {
+ ownerManager.setOwner(
+ ident.name(),
Review Comment:
(Pre-existing, unrelated to this PR) The first argument to `setOwner` should
be the metalake name (`ident.namespace().level(0)`), not the model name
(`ident.name()`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
