In-memory Connector #1262
In-memory Connector #1262
Conversation
Conformance comparison report
Number passing in both: 5372 Number failing in both: 446 Number passing in Base (3f3bb7b) but now fail: 0 Number failing in Base (3f3bb7b) but now pass: 0 |
|
Pause this work. The issue is related to local plugin. The local plugin attempts retrieve catalog/schema via In local build the files system approach works fine as we can directly access the file via URI. For some reasons, in GitHub work flow, the build runs jar instead of a local file system. We could not convert a folder in Jar to Java File Object. TODO:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## partiql-planner #1262 +/- ##
==================================================
Coverage ? 72.01%
Complexity ? 2058
==================================================
Files ? 221
Lines ? 15872
Branches ? 2853
==================================================
Hits ? 11431
Misses ? 3634
Partials ? 807
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| val catalogProvider = MemoryCatalog.Provider().also { | ||
| val map = mutableMapOf<String, MutableList<Pair<String, StaticType>>>() | ||
| inputStream.reader().readLines().forEach { path -> | ||
| if (path.startsWith("catalogs/default")) { | ||
| val schema = this::class.java.getResourceAsStream("/$path")!! | ||
| val ion = loadSingleElement(schema.reader().readText()) | ||
| val staticType = ion.toStaticType() | ||
| val steps = path.split('/').drop(2) // drop the catalogs/default | ||
| val catalogName = steps.first() | ||
| val subPath = steps | ||
| .drop(1) | ||
| .joinToString(".") { it.lowercase() } | ||
| .let { | ||
| it.substring(0, it.length - 4) | ||
| } | ||
| if (map.containsKey(catalogName)) { | ||
| map[catalogName]!!.add(subPath to staticType) | ||
| } else { | ||
| map[catalogName] = mutableListOf(subPath to staticType) | ||
| } | ||
| } | ||
| } | ||
| map.forEach { (k: String, v: MutableList<Pair<String, StaticType>>) -> | ||
| it[k] = MemoryCatalog.of(*v.toTypedArray()) | ||
| } | ||
| } |
There was a problem hiding this comment.
I see, rather than porting the catalogs from the .ion files you create the mem catalog from them. This works, but would be nice to clean up the resources in a later PR. Again, these are testing concerns so nothing pressing
| // TODO: This code is ported from plugins/partiql-local/src/main/kotlin/org/partiql/plugins/local/LocalSchema.kt | ||
| // In my opinion, the in-memory connector should be independent of schema file format, | ||
| // hence making it inappropriate to leave the code in plugins/partiql-memory | ||
| // We need to figure out where to put the code. |
There was a problem hiding this comment.
I think moving the catalogs from resources to in-memory is a better long term solution and don't need to duplicate this.
Relevant Issues
Description
Notes on CI builds:
Fileobject later.Fix : https://github.com/actions/checkout/blob/v3.0.2/README.md#user-content-checkout-pull-request-head-commit-instead-of-merge-commit
Commits:
resource_path.txtfile during build process, and use the file to populate the in-memory connector. Also, currently we have a couple test cases that are failing due to theANYfunction resolution in plan. I added aIgnoredTestCaseto workaround this issue.Other Information
Updated Unreleased Section in CHANGELOG: [YES/NO]
Any backward-incompatible changes? [YES/NO]
Any new external dependencies? [YES/NO]
Do your changes comply with the Contributing Guidelines
and Code Style Guidelines? [YES/NO]
Yes.
License Information
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.