Support Metadata trait #1096

devgony · 2023-02-18T05:29:05Z

Goal

Support Metadata trait
Metadata uses Schemaless so that storage developer can put anything in it but basically it contains CREATED field of Table and Index

Todo

Implement Metadata trait
remove CREATED from GLUE_OBJECTS of core
split test case of CREATED to generate_metadata_tests

Remove Store trait related cfg features, (gluesql#1091) Enable all by default. Compile time feature control causes too much management cost. We already have test cases in test-suite splitted based on store trait features. Support `values` query in AST builder (gluesql#1041) Currently, ASTBuilder does not support Values Query yet. This PR supports Values Query in ASTBuilder e.g. let actual = values(vec!["1, 'a'", "2, 'b'"]) .order_by(vec!["column1 desc"]) .build(); let expected = "VALUES(1, 'a'), (2, 'b') ORDER BY column1 desc";

coveralls · 2023-03-06T05:16:03Z

Pull Request Test Coverage Report for Build 4586977286

97 of 98 (98.98%) changed or added relevant lines in 8 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage increased (+2.0e**-05%**) to 98.858%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
core/src/executor/fetch.rs	25	26	96.15%

Files with Coverage Reduction	New Missed Lines	%
core/src/executor/validate.rs	1	99.24%

Totals
Change from base Build 4552438461:	2.0e-05%
Covered Lines:	40605
Relevant Lines:	41074

💛 - Coveralls

panarch · 2023-03-11T07:56:20Z

storages/memory-storage/src/lib.rs

@@ -68,21 +72,52 @@ impl Store for MemoryStorage {
    }
 }

+#[async_trait(?Send)]
+impl Metadata for MemoryStorage {
+    async fn scan_meta(&self) -> HashMap<String, Value> {


async function scan_metadata(&self) -> Result<Box<dyn Iterator<Item = Result<(String, Value)>>>>

it looks that we can simply use HashMap<String, Value> rather than Value.

…Iter

panarch · 2023-03-18T10:00:06Z

storages/memory-storage/src/lib.rs

 pub struct MemoryStorage {
    pub id_counter: i64,
    pub items: HashMap<String, Item>,
+    pub metadata: HashMap<String, Value>,


pub metadata: HashMap<String, HashMap<String, Value>>,

this would be more precise..?

panarch · 2023-03-18T10:03:10Z

test-suite/src/metadata/mod.rs

+pub mod table;
+pub use table::table;


These can provide two different ways to use module.

Like other modules we are using,
ref. https://github.com/gluesql/gluesql/blob/main/test-suite/src/data_type/mod.rs

How about only using pub mod table;?

panarch · 2023-03-18T10:04:40Z

core/src/store/mod.rs


 pub trait GStoreMut: StoreMut + IndexMut + AlterTable + Transaction {}
 impl<S: StoreMut + IndexMut + AlterTable + Transaction> GStoreMut for S {}

 pub use {
+    self::metadata::{MetaIter, Metadata},


Like other modules, alter_table, data_row ..
we can omit self::

…ring, Value>>

panarch · 2023-03-18T11:32:06Z

core/src/executor/fetch.rs

+                            let meta = table_metas.iter().find_map(|(table_name, hash_map)| {
+                                if table_name == &schema.table_name {
+                                    Some(Value::Map(hash_map.clone()))
+                                } else {
+                                    None
+                                }
+                            });
+                            let table_row = HashMap::from([
+                                ("OBJECT_NAME".to_owned(), Value::Str(schema.table_name)),
+                                ("OBJECT_TYPE".to_owned(), Value::Str("TABLE".to_owned())),
+                            ]);
+
+                            let table_rows = match meta {
+                                Some(Value::Map(meta)) => {
+                                    table_row.into_iter().chain(meta).collect()
+                                }
+                                _ => table_row,
+                            };


It looks that we can reduce use of collect.

let meta = table_metas .iter() .find_map(|(table_name, hash_map)| { (table_name == &schema.table_name).then(|| hash_map.clone()) }) .unwrap_or_default(); let table_rows = [ ("OBJECT_NAME".to_owned(), Value::Str(schema.table_name)), ("OBJECT_TYPE".to_owned(), Value::Str("TABLE".to_owned())), ] .into_iter() .chain(meta);

panarch · 2023-03-18T11:36:45Z

storages/memory-storage/src/lib.rs

@@ -71,18 +75,30 @@ impl Store for MemoryStorage {
 #[async_trait(?Send)]
 impl StoreMut for MemoryStorage {
    async fn insert_schema(&mut self, schema: &Schema) -> Result<()> {
+        let created = HashMap::from([
+            ("OBJECT_TYPE".to_owned(), Value::Str("TABLE".to_owned())),


This would not be necessary because which the same OBJECT_TYPE property is being used in execution layer.

I removed OBJECT_TYPE in this PR but it may be needed if we implement metadata of index later since only OBJECT_NAME is used as a key here.
Just in case TABLE_NAME and INDEX_NAME are identical (but object_types are different), both OBJECT_NAME and OBJECT_TYPE should be a key
Or need some separated stored area between table and index

panarch

This is great!
All looks nice 👍 Thanks a lot 👍 👍

devgony self-assigned this Mar 3, 2023

devgony force-pushed the metadata-trait branch from ba609b2 to ced0d9d Compare March 6, 2023 04:57

devgony added 7 commits March 6, 2023 14:10

feat: implement Metadata base

a3b06fb

feat: implement scan_meta base

1557dc5

fix: minimize implementation (created only) + with schemaless

0949b20

feat: implement fetch glue_object base

3219089

refactor: remove CREATED from test-suit

dd475e8

fix: add importing Metadata

a2fb218

fix: merge main

1dc5f48

devgony force-pushed the metadata-trait branch from 8cfc919 to 1dc5f48 Compare March 6, 2023 05:20

devgony added 3 commits March 6, 2023 21:03

refactor: split test to generate_metadata_tests!

0d0f3af

fix: fix wrong merge

7ca6ef1

refactor: lighten table_rows

7bd7db0

devgony marked this pull request as ready for review March 11, 2023 07:43

devgony requested review from panarch and ever0de and removed request for panarch March 11, 2023 07:43

devgony added the enhancement New feature or request label Mar 11, 2023

ever0de requested a review from panarch March 11, 2023 07:44

panarch reviewed Mar 11, 2023

View reviewed changes

devgony added 5 commits March 11, 2023 19:58

fix: remove unneeded delete_meta, append_meta; scan_meta returns Meta…

0fa8c41

…Iter

refactor: remove mut and collect

cb6ef4d

refactor: move Metadata from store/mod.rs to metadata.rs

3428634

chore: remove comments

776d0d9

refactor: rename TableName to ObjectName

8830332

panarch self-requested a review March 18, 2023 08:14

panarch reviewed Mar 18, 2023

View reviewed changes

devgony added 6 commits March 18, 2023 19:05

refactor: split memory-storage::metadata.rs

f76124b

refactor: use more precise type: metadata: HashMap<String, HashMap<St…

7c25f9b

…ring, Value>>

refactor: remove redundant module. use only pub mod table

e2dd1b6

refactor: omit self

5319add

fix: scan only table metadata

233a184

style: thanks clippy

c16c58f

panarch reviewed Mar 18, 2023

View reviewed changes

devgony added 6 commits March 18, 2023 20:54

feat: add metadata_index_test

56a8933

fix: move flat_map to outer iteration

ffef08c

fix: remove unneeded OBJECT_TYPE

c641ddb

style: thanks clippy

4a0be05

Merge remote-tracking branch 'upstream/main' into metadata-trait

8a557ec

fix: correct name to JsonStorage

91fcbfb

panarch self-requested a review April 2, 2023 05:11

panarch approved these changes Apr 2, 2023

View reviewed changes

panarch merged commit a249626 into gluesql:main Apr 2, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Metadata trait #1096

Support Metadata trait #1096

devgony commented Feb 18, 2023 •

edited

coveralls commented Mar 6, 2023 •

edited

panarch Mar 11, 2023

panarch Mar 11, 2023

panarch Mar 18, 2023

panarch Mar 18, 2023

panarch Mar 18, 2023

panarch Mar 18, 2023

panarch Mar 18, 2023

devgony Mar 19, 2023 •

edited

panarch left a comment

Support Metadata trait #1096

Support Metadata trait #1096

Conversation

devgony commented Feb 18, 2023 • edited

Goal

Todo

coveralls commented Mar 6, 2023 • edited

Pull Request Test Coverage Report for Build 4586977286

💛 - Coveralls

panarch Mar 11, 2023

Choose a reason for hiding this comment

panarch Mar 11, 2023

Choose a reason for hiding this comment

panarch Mar 18, 2023

Choose a reason for hiding this comment

panarch Mar 18, 2023

Choose a reason for hiding this comment

panarch Mar 18, 2023

Choose a reason for hiding this comment

panarch Mar 18, 2023

Choose a reason for hiding this comment

panarch Mar 18, 2023

Choose a reason for hiding this comment

devgony Mar 19, 2023 • edited

Choose a reason for hiding this comment

panarch left a comment

Choose a reason for hiding this comment

devgony commented Feb 18, 2023 •

edited

coveralls commented Mar 6, 2023 •

edited

devgony Mar 19, 2023 •

edited