cecemei commented on code in PR #17774:
URL: https://github.com/apache/druid/pull/17774#discussion_r2049501795
##########
processing/src/main/java/org/apache/druid/segment/SegmentReference.java:
##########
@@ -19,12 +19,31 @@
package org.apache.druid.segment;
+import org.apache.druid.query.policy.NoopPolicyEnforcer;
+import org.apache.druid.query.policy.PolicyEnforcer;
+
/**
- * A {@link Segment} with a associated references, such as {@link
ReferenceCountingSegment} where the reference is
+ * A {@link Segment} with an associated references, such as {@link
ReferenceCountingSegment} where the reference is
* the segment itself, and {@link
org.apache.druid.segment.join.HashJoinSegment} which wraps a
* {@link ReferenceCountingSegment} and also includes the associated list of
* {@link org.apache.druid.segment.join.JoinableClause}
*/
public interface SegmentReference extends Segment, ReferenceCountedObject
{
+
+ /**
+ * Validates if the segment complies with the policy restrictions on tables.
+ * <p>
+ * This should be called right before the segment is about to be processed
by the query stack, and after
+ * {@link
org.apache.druid.query.planning.ExecutionVertex#createSegmentMapFunction(PolicyEnforcer)}.
+ */
+ default void validateOrElseThrow(PolicyEnforcer policyEnforcer)
Review Comment:
Thanks for the example, I was able to reproduce the error now, it looks like
right child is inlined but it has already gone through a scan stage, and the
current implementation missed the check on the left child.
I'm think, we can ask `BroadcastJoinSegmentMapFnProcessor` to create an
`ExecutionVertex` based on the query, and pass enforcer in `ExecutionVertex`'s
`createSegmentMapFunction`, so like
```
DataSource transformed = inlineChannelData(query.getDataSource());
ExecutionVertex ev = ExecutionVertex.of(query.withDataSource(transformed));
return ev.createSegmentMapFunction(policyEnforcer);
```
This way we can centralize the enforcer in `ExecutionVertex` without
changing `DataSource` interface.
For the Dart issue, I didn't know it can wrap two layer of
`ReferenceCountingSegment`, I think maybe we can add recursive in the
validation? For a restricted segment, would it be
- Restrict
- Ref
- Ref_base
- policy
or
- Ref
- Restrict
- Ref_base
- policy
?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
