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

Fix property graph case-sentisitivity #188

Merged
merged 4 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/core/functions/scalar/local_clustering_coefficient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ static void LocalClusteringCoefficientFunction(DataChunk &args,
int64_t count = 0;
for (int64_t offset = v[src_node]; offset < v[src_node + 1]; offset++) {
int64_t neighbor = e[offset];
for (int64_t offset2 = v[neighbor]; offset2 < v[neighbor + 1]; offset2++) {
for (int64_t offset2 = v[neighbor]; offset2 < v[neighbor + 1];
offset2++) {
int is_connected = neighbors.test(e[offset2]);
count += is_connected; // Add 1 if connected, 0 otherwise
}
Expand Down
3 changes: 2 additions & 1 deletion src/core/functions/table/describe_property_graph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,8 @@ void DescribePropertyGraphFunction::DescribePropertyGraphFunc(
output.SetValue(12, vector_idx, Value());
} else {
output.SetValue(12, vector_idx, Value(edge_table->catalog_name));
} output.SetValue(13, vector_idx, Value(edge_table->schema_name));
}
output.SetValue(13, vector_idx, Value(edge_table->schema_name));
vector_idx++;
}
output.SetCardinality(vector_idx);
Expand Down
9 changes: 5 additions & 4 deletions src/core/pragma/show_property_graphs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@ namespace duckpgq {

namespace core {

static string PragmaShowPropertyGraphs(ClientContext &context, const FunctionParameters &parameters) {
static string PragmaShowPropertyGraphs(ClientContext &context,
const FunctionParameters &parameters) {
return "SELECT DISTINCT property_graph from __duckpgq_internal";
}

void CorePGQPragma::RegisterShowPropertyGraphs(DatabaseInstance &instance) {
// Define the pragma function
auto pragma_func = PragmaFunction::PragmaCall(
"show_property_graphs", // Name of the pragma
PragmaShowPropertyGraphs, // Query substitution function
{} // Parameter types (mail_limit is an integer)
"show_property_graphs", // Name of the pragma
PragmaShowPropertyGraphs, // Query substitution function
{} // Parameter types (mail_limit is an integer)
);

// Register the pragma function
Expand Down
1 change: 1 addition & 0 deletions src/duckpgq_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ void DuckPGQState::PopulateEdgeSpecificFields(unique_ptr<DataChunk> &chunk,
void DuckPGQState::ExtractListValues(const Value &list_value,
vector<string> &output) {
auto children = ListValue::GetChildren(list_value);
output.reserve(output.size() + children.size());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reserve memory before emplace.

for (const auto &child : children) {
output.push_back(child.GetValue<string>());
}
Expand Down
3 changes: 0 additions & 3 deletions src/include/duckpgq/core/pragma/duckpgq_pragma.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,8 @@ class CorePGQPragma {

private:
static void RegisterShowPropertyGraphs(DatabaseInstance &instance);

};



} // namespace core

} // namespace duckpgq
3 changes: 2 additions & 1 deletion src/include/duckpgq_state.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "duckpgq/common.hpp"
#include "duckdb/common/case_insensitive_map.hpp"

#include <duckpgq/core/utils/compressed_sparse_row.hpp>

Expand Down Expand Up @@ -33,7 +34,7 @@ class DuckPGQState : public ClientContextState {
// unnamed graph tables

//! Property graphs that are registered
std::unordered_map<string, unique_ptr<CreateInfo>> registered_property_graphs;
case_insensitive_map_t<unique_ptr<CreateInfo>> registered_property_graphs;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only change, all others are made by linters via make format.


//! Used to build the CSR data structures required for path-finding queries
std::unordered_map<int32_t, unique_ptr<duckpgq::core::CSR>> csr_list;
Expand Down
18 changes: 18 additions & 0 deletions test/sql/create_pg/create_property_graph.test
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,24 @@ EDGE TABLES (
PROPERTIES ( createDate ) LABEL Knows
)

query I
-SELECT count(id)
FROM
GRAPH_TABLE (PG
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The property graph created at L43 is of lower case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to double check, could you change the test to verify it has the right result? Instead of only checking that the query doesn't crash :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the PR description to reflect the manual test result, which looks ok to me.

Let me check how to leverage the testing framework to verify the output as well.
I feel confused at the first place, since other SELECT statements are also annotated as statement ok.

statement ok
SELECT * FROM get_csr_ptr(0);

Let me check and get back to you. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated! PTAL, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! LGTM

MATCH p = (s1:Person)-[k:Knows]->(s2:Person WHERE s2.name='Daniel')
COLUMNS (s1.id));
----
0

query I
-SELECT count(id)
FROM
GRAPH_TABLE (PG
MATCH p = (s1:Person)-[k:Knows]->(s2:Person WHERE s2.name='Peter')
COLUMNS (s1.id));
----
3

# Error as property graph pg already exists
statement error
-CREATE PROPERTY GRAPH pg
Expand Down
Loading