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

Create External Table without schema [Spark] #168

Merged
merged 4 commits into from
Mar 3, 2023

Conversation

osopardo1
Copy link
Member

Description

This PR fixes #165

Type of change

When creating an external table with an already populated directory, Qbeast was missing the schema in the final Catalog entry.
We add a method to detect this specific situation and update the service accordingly.

Checklist:

Here is the list of things you should do before submitting this pull request:

  • New feature / bug fix has been committed following the Contribution guide.
  • Add comments to the code (make it easier for the community!).
  • Add tests.
  • Your branch is updated to the main branch (dependent changes have been merged).

How Has This Been Tested? (Optional)

A new test is added in QbeastCatalogIntegrationTest in which we try to create an external table without schema.

@osopardo1
Copy link
Member Author

We need to test this solution on the EMR clusters with Glue Catalog, see if we miss something else in the implementation and if everything works correctly.

@osopardo1 osopardo1 changed the title 165 create external table Create External Table without schema [Spark] Feb 23, 2023
@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

Merging #168 (318ee28) into main (a425a22) will increase coverage by 0.00%.
The diff coverage is 94.11%.

❗ Current head 318ee28 differs from pull request most recent head b49b63b. Consider uploading reports for the commit b49b63b to get more accurate results

@@           Coverage Diff           @@
##             main     #168   +/-   ##
=======================================
  Coverage   93.71%   93.71%           
=======================================
  Files          81       81           
  Lines        1925     1941   +16     
  Branches      145      154    +9     
=======================================
+ Hits         1804     1819   +15     
- Misses        121      122    +1     
Impacted Files Coverage Δ
.../internal/sources/catalog/QbeastCatalogUtils.scala 96.42% <94.11%> (-0.64%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@osopardo1
Copy link
Member Author

Hello @Adricu8 !! Do you think this is ready to merge? Did you find any bug creating external tables with Qbeast? (We still missing pre-load the columnsToIndex too, but we will develop it in another PR).

@osopardo1 osopardo1 marked this pull request as ready for review March 2, 2023 08:19
@osopardo1 osopardo1 requested a review from Adricu8 March 2, 2023 08:19
@Adricu8
Copy link
Contributor

Adricu8 commented Mar 2, 2023

Hi Paola! I did not find any issue with this. Creating external tables worked fine for me :)

@osopardo1
Copy link
Member Author

Hi Paola! I did not find any issue with this. Creating external tables worked fine for me :)

Perfect! Could you approve the PR? 🙏

Copy link
Contributor

@Adricu8 Adricu8 left a comment

Choose a reason for hiding this comment

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

Changes look good to me, thanks Paola :)

@osopardo1 osopardo1 merged commit 074e5e9 into Qbeast-io:main Mar 3, 2023
@osopardo1 osopardo1 deleted the 165-create-external-table branch August 2, 2023 05:52
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.

CREATE EXTERNAL TABLE does not save schema [Spark]
2 participants