ENG-1012 Update Query Builder Relation Conditions to Use Reified Relation Triples (#538)

* eng-1012 update query builder to use reified relations

Author: maparent

apps/roam/package.json

@@ -68,7 +68,7 @@
     "react-draggable": "4.4.5",
     "react-in-viewport": "1.0.0-alpha.20",
     "react-vertical-timeline-component": "3.5.2",
-    "roamjs-components": "0.85.4",
+    "roamjs-components": "0.85.6",
     "tldraw": "2.3.0",
     "use-sync-external-store": "1.5.0",
     "xregexp": "^5.0.0",

apps/roam/src/utils/createReifiedBlock.ts

@@ -5,10 +5,19 @@ import { getSetting } from "~/utils/extensionSettings";
 
 export const DISCOURSE_GRAPH_PROP_NAME = "discourse-graph";
 
+const SANE_ROLE_NAME_RE = new RegExp(/^[\w\-]*$/);
+
 const strictQueryForReifiedBlocks = async (
   parameterUids: Record<string, string>,
 ): Promise<string | null> => {
   const paramsAsSeq = Object.entries(parameterUids);
+  // validate parameter names
+  if (
+    Object.keys(parameterUids).filter((k) => !k.match(SANE_ROLE_NAME_RE)).length
+  )
+    throw new Error(
+      `invalid parameter names in ${Object.keys(parameterUids).join(", ")}`,
+    );
   const query = `[:find ?u ?d
   :in $ ${paramsAsSeq.map(([k]) => "?" + k).join(" ")}
   :where [?s :block/uid ?u] [?s :block/props ?p] [(get ?p :${DISCOURSE_GRAPH_PROP_NAME}) ?d]
@@ -72,7 +81,7 @@ const createReifiedBlock = async ({
 const RELATION_PAGE_TITLE = "roam/js/discourse-graph/relations";
 let relationPageUid: string | undefined = undefined;
 
-const getRelationPageUid = async (): Promise<string> => {
+const getOrCreateRelationPageUid = async (): Promise<string> => {
   if (relationPageUid === undefined) {
     relationPageUid = getPageUidByPageTitle(RELATION_PAGE_TITLE);
     if (relationPageUid === "") {
@@ -82,8 +91,16 @@ const getRelationPageUid = async (): Promise<string> => {
   return relationPageUid;
 };
 
+export const getExistingRelationPageUid = (): string | undefined => {
+  if (relationPageUid === undefined) {
+    const uid = getPageUidByPageTitle(RELATION_PAGE_TITLE);
+    if (uid !== "") relationPageUid = uid;
+  }
+  return relationPageUid;
+};
+
 export const countReifiedRelations = async (): Promise<number> => {
-  const pageUid = await getRelationPageUid();
+  const pageUid = getExistingRelationPageUid();
   if (pageUid === undefined) return 0;
   const r = await window.roamAlphaAPI.data.async.q(
     `[:find (count ?c) :where [?p :block/children ?c] [?p :block/uid "${pageUid}"]]`,
@@ -103,7 +120,7 @@ export const createReifiedRelation = async ({
   const authorized = getSetting("use-reified-relations");
   if (authorized) {
     return await createReifiedBlock({
-      destinationBlockUid: await getRelationPageUid(),
+      destinationBlockUid: await getOrCreateRelationPageUid(),
       schemaUid: relationBlockUid,
       parameterUids: {
         sourceUid,

apps/roam/src/utils/fireQuery.ts

@@ -1,8 +1,8 @@
 import conditionToDatalog from "./conditionToDatalog";
 import type {
   PullBlock,
-  DatalogAndClause,
   DatalogClause,
+  DatalogAndClause,
 } from "roamjs-components/types";
 import compileDatalog from "./compileDatalog";
 import { getNodeEnv } from "roamjs-components/util/env";
@@ -65,9 +65,9 @@ const firstVariable = (
 };
 
 const optimizeQuery = (
-  clauses: (DatalogClause | DatalogAndClause)[],
+  clauses: DatalogClause[],
   capturedVariables: Set<string>,
-): (DatalogClause | DatalogAndClause)[] => {
+): DatalogClause[] => {
   const marked = clauses.map(() => false);
   const orderedClauses: (DatalogClause | DatalogAndClause)[] = [];
   const variablesByIndex: Record<number, Set<string>> = {};
@@ -107,7 +107,8 @@ const optimizeQuery = (
         if (Array.from(allVars).every((v) => capturedVariables.has(v))) {
           score = 10;
         } else {
-          score = 100002;
+          // downgrade disjunction and negation
+          score = c.type === "and-clause" ? 100002 : 100006;
         }
       } else if (c.type === "not-join-clause" || c.type === "or-join-clause") {
         if (c.variables.every((v) => capturedVariables.has(v.value))) {
@@ -125,7 +126,8 @@ const optimizeQuery = (
             (a) => a.type !== "variable" || capturedVariables.has(a.value),
           )
         ) {
-          score = 1000;
+          // equality is almost as good as a binding
+          c.type == "pred-expr" && c.pred == "=" ? (score = 5) : (score = 1000);
         } else {
           score = 100004;
         }
@@ -155,6 +157,16 @@ const optimizeQuery = (
       bestClause.arguments
         .filter((v) => v.type === "variable")
         .forEach((v) => capturedVariables.add(v.value));
+    } else if (bestClause.type === "fn-expr") {
+      // A function expression acts as biding a variable to a unique function value
+      if (
+        bestClause.arguments.filter(
+          (a) => a.type === "variable" && !capturedVariables.has(a.value),
+        ).length === 0 &&
+        bestClause.binding.type === "bind-scalar" &&
+        bestClause.binding.variable.type === "variable"
+      )
+        capturedVariables.add(bestClause.binding.variable.value);
     }
   }
   return orderedClauses;
@@ -197,7 +209,7 @@ export const getDatalogQuery = ({
   const whereClauses = optimizeQuery(
     getWhereClauses({ conditions, returnNode }),
     new Set([]),
-  ) as DatalogClause[];
+  );
 
   const defaultSelections: {
     mapper: PredefinedSelection["mapper"];