Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add predicate pushdown (updated) #78

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mike-luabase
Copy link

No description provided.

@mike-luabase mike-luabase changed the title Add predicate pushdown4 Add predicate pushdown (updated) Oct 16, 2024
select_statement->node = std::move(select_node);
return make_uniq<SubqueryRef>(std::move(select_statement), "iceberg_scan");
vector<Value> structs;
for (const auto &file : data_file_values) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The majority of lines changed here are still formatting changes - could you format using our clang-format config?

Copy link
Author

Choose a reason for hiding this comment

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

@Mytherin fixed the formatting, sorry about that!

@samansmink
Copy link
Collaborator

Also @mike-luabase, you don't need to reopen the PR every time, this makes it harder to review actually since we lose a clear view on the review comments that were made before.

If you want to overwrite commits you can just force push to your feature branch

@mike-luabase
Copy link
Author

Also @mike-luabase, you don't need to reopen the PR every time, this makes it harder to review actually since we lose a clear view on the review comments that were made before.

Yes, understood, won't do that again.

@mike-luabase
Copy link
Author

This test is failing, looking into it

SELECT count(*) FROM ICEBERG_SCAN('data/iceberg/generated_spec2_0_001/pyspark_iceberg_table');
Lower bounds is null
Upper bounds is null
Binder Error: Table "iceberg_scan_data" does not have a column named "filename"

@mike-luabase
Copy link
Author

@Mytherin test failure should be fixed now

@mike-luabase
Copy link
Author

@samansmink @Mytherin can we trigger the run again? This error doesn't seem related to my changes:

#7 [ 3/14] RUN apt-get install -y -qq software-properties-common
#7 0.232 E: Unable to locate package software-properties-common
#7 ERROR: process "/bin/sh -c apt-get install -y -qq software-properties-common" did not complete successfully: exit code: 100
------
 > [ 3/14] RUN apt-get install -y -qq software-properties-common:
0.232 E: Unable to locate package software-properties-common
------
Dockerfile:9
--------------------
   7 |     # Setup the basic necessities
   8 |     RUN apt-get update -y -qq
   9 | >>> RUN apt-get install -y -qq software-properties-common
  10 |     RUN apt-get install -y -qq --fix-missing ninja-build make gcc-multilib g++-multilib libssl-dev wget openjdk-8-jdk zip maven unixodbc-dev libc6-dev-i386 lib32readline6-dev libssl-dev libcurl4-gnutls-dev libexpat1-dev gettext unzip build-essential checkinstall libffi-dev curl libz-dev openssh-client pkg-config autoconf
  11 |     RUN apt-get install -y -qq ccache
--------------------
ERROR: failed to solve: process "/bin/sh -c apt-get install -y -qq software-properties-common" did not complete successfully: exit code: 100

@catkins
Copy link

catkins commented Oct 18, 2024

@mike-luabase 💚 I'm so excited to see movement on this! I'll be able to delete a bunch of manual partition pruning + read_parquet code in an app of mine!

@mike-luabase
Copy link
Author

@samansmink anything I can do to help here?

@samansmink
Copy link
Collaborator

If you could re-add a PR description and address @Mytherin's comment from one of your previous PRs #75 (comment), I will take a more detailed look to review this.

Please be considerate that reviews take a lot of time, especially reviews from outside contributors that are touching complicated code. To get your PRs through as quick as possible, I recommend you to:

  • double check your ideas for a PR with a core contributor of DuckDB through either the DuckDB discord channel, a github issue or a github discussion. See (https://github.com/duckdb/duckdb/blob/main/CONTRIBUTING.md)
  • Make sure that your PRs are easy to review
    • no big PRs
    • don't add big format changes to your PRs
    • write a clear description of what you are adding and why you chose the path you chose

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants