carloea2 commented on PR #6016:
URL: https://github.com/apache/texera/pull/6016#issuecomment-4828996142
Thanks, this is much more focused now.
The core fix looks right to me: filtering `DATASET_USER_ACCESS` by the
current `uid` in the JOIN should prevent the public-dataset join fan-out, since
`dataset_user_access` is keyed by `(did, uid)`.
I have two requests:
1. Please add a null-safe fallback for `accessPrivilege`.
After this change, public datasets without an explicit access row for the
current user will have no joined `DATASET_USER_ACCESS.PRIVILEGE`, so
`record.get(...)` can be null. The frontend type expects `"READ" | "WRITE" |
"NONE"`, so I think we should return `PrivilegeEnum.NONE` instead of null.
Example:
val accessPrivilege =
Option(record.get(DATASET_USER_ACCESS.PRIVILEGE,
classOf[PrivilegeEnum]))
.getOrElse(PrivilegeEnum.NONE)
2. Please add a small spec for the duplicate case:
- dataset is public
- dataset is explicitly shared with the current user
- search with `includePublic=true`
- result contains exactly one dataset row, with the current user's
privilege
Optional cleanup: the condition rewrite is logically okay, but the PR could
be smaller if we keep the existing WHERE logic and only change the JOIN. Also,
the new comments have a couple typos and “join is skipped” is slightly
misleading because the join remains present but is forced to match no rows when
`uid == null`.
--
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]
