rdblue commented on a change in pull request #1353:
URL: https://github.com/apache/iceberg/pull/1353#discussion_r473449341



##########
File path: core/src/main/java/org/apache/iceberg/BaseTableScan.java
##########
@@ -159,6 +159,10 @@ public TableScan option(String property, String value) {
 
   @Override
   public TableScan project(Schema projectedSchema) {
+    if (context.selectedColumns() != null) {
+      throw new IllegalStateException("Cannot project schema when selected 
columns is specified.");
+    }

Review comment:
       Thanks @edgarRd! I think it makes sense to move that into the context 
and initialize it as null. If the projection is null, then use the column list. 
If not, then use the projection. And then both methods can validate that the 
other has not been called.
   
   I think it would be okay to do this even if it only works for 
`DataTableScan`. If the other scans don't support it, then they should throw 
exceptions instead of allowing the projection to be set.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to