Skip to content

Add support for CreateScan and GetScanTasks in RESTCatalog#1

Open
rahil-c wants to merge 1 commit intomainfrom
rchertar/main
Open

Add support for CreateScan and GetScanTasks in RESTCatalog#1
rahil-c wants to merge 1 commit intomainfrom
rchertar/main

Conversation

@rahil-c
Copy link
Copy Markdown
Owner

@rahil-c rahil-c commented Dec 5, 2023

No description provided.

import org.apache.iceberg.util.ArrayUtil;

abstract class BaseContentScanTask<ThisT extends ContentScanTask<F>, F extends ContentFile<F>>
public abstract class BaseContentScanTask<
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it necessary to make this public? can we avoid changing this?

return deletesSizeBytes;
}

@VisibleForTesting
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid changing this

Comment thread core/src/main/java/org/apache/iceberg/FileScanTaskParser.java
String filterString = ExpressionParser.toJson(filter());

CreateScanRequest request =
CreateScanRequest.builder().withselect(columnNames()).withfilter(filterString).build();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the capitalization seems wrong, should be withSelect, withFilter, etc.

return select;
}

public String filter() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should set Expression, but not String. The request parser should parse it to string.

return filter;
}

public Map<String, String> options() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove this? we don't need this for OSS

return this;
}

public Builder withshard(String newshard) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withShard

Comment thread core/src/main/java/org/apache/iceberg/rest/RESTSerializers.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants