fluree / db

Fluree database library
https://fluree.github.io/db/
Other
340 stars 22 forks source link

[v2] `vars` map misinterprets two-tuples as subject identifiers instead of two possible values #479

Closed aaj3f closed 1 year ago

aaj3f commented 1 year ago

Description

This v2 bug fix is being prioritized because it affects a client under contract

When using a vars map to provide multiple value options for a particular binding, it will work with one option or 3+ options, but if you provide two options (i.e. a two-tuple), Fluree tries to resolve that as a pred-object identifier for a particular subject, e.g.

{
    ...,
    "vars": {
        "?achievementType": ["Degree", "Course"]
    },
    ...

=>

{
  "status": 400,
  "message": "Error looking up subject id: [\"Achievement\" \"Course\"]",
  "error": "db/invalid-subject"
}
cap10morgan commented 1 year ago

It appears that supporting two-tuple subject identifiers is intentional (see test here: https://github.com/fluree/ledger/blob/fdfefe942848559043abcf883b5f4da32a7773c4/test/fluree/db/ledger/query/analytical_test.clj#L171-L186), so we may need to decide to either break that or to add support for an additional disambiguating syntax for two-value vars.

cap10morgan commented 1 year ago

Some options:

  1. If I were designing this from scratch or didn't care about backwards compatibility I would suggest requiring a two-dimensional vector / array when using two-tuple subject identifiers so that the outer one is always unambiguously a "values" vector. So that test I linked above would have to become [["_user/username" "jake"]].
  2. The best backwards-compatible option I've come up with is to wrap value arrays in an object like this: {"anyOf": ["can't be a two-tuple", "subject identifier"]}.
  3. I suppose we could query for whether or not the first element of a two-element array is an existing predicate or not. But this feels brittle.

Any other ideas?

aaj3f commented 1 year ago

@cap10morgan I agree that Option 1 would be ideal. I know breaking backwards compatibility is generally verboten, but I do think support for multi-cardinality on vars is basically brand-new to v2: it was one of the last net new features that we implemented, afaicr.

I'd appreciate any input from @bplatz, @zonotope or others, but my opinion would be that the vars map is basically broken for multi-cardinality options unless it can handle not only 1-tuples or 3+-tuples but also 2-tuples, and so I feel like moving to a pattern of [8888888, ["two-tuple", "identifier"], 9999999, 0000000] is more of a necessary significant fix than a feature redesign.

zonotope commented 1 year ago

@cap10morgan I agree that Option 1 would be ideal. I know breaking backwards compatibility is generally verboten, but I do think support for multi-cardinality on vars is basically brand-new to v2: it was one of the last net new features that we implemented, afaicr.

I'd appreciate any input from @bplatz, @zonotope or others, but my opinion would be that the vars map is basically broken for multi-cardinality options unless it can handle not only 1-tuples or 3+-tuples but also 2-tuples, and so I feel like moving to a pattern of [8888888, ["two-tuple", "identifier"], 9999999, 0000000] is more of a necessary significant fix than a feature redesign.

I agree that option 1 should be preferred, and this is essentially how values are handled in 3.0 (besides a slight syntax change).

cap10morgan commented 1 year ago

OK sounds like we have a consensus path forward then 👍

cap10morgan commented 1 year ago

@aaj3f I'm having trouble replicating the working scenario with three bindings. Can you provide a more complete working example of that?

aaj3f commented 1 year ago

@cap10morgan I can provide the example of the query and results, but would you prefer that I actually provide a zipped data directory with the data & query that would allow you to test against this?

Here's the query:

{
    "selectDistinct": {
        "?assertion": ["*", {"achievement": ["*"]}]
    },
    "where": [
      ["?assertion", "assertion/achievement", "?achievement"],
      ["?achievement", "achievement/type", "?achievementType"]
    ],
    "vars": {
        "?achievementType": ["Degree", "Course", "Credential"]
    },
    "opts": {
      "compact": true,
      "limit": 100,
      "offset": 0
    }
}

And the query results

[
  {
    "_id": 351843720888420,
    "results": [
      {
        "_id": 369435906932739
      }
    ],
    "id": "B_AA_DEGREE_01",
    "term": "2022 Spring",
    "recipient": {
      "_id": 422212465065995
    },
    "source": {
      "_id": 404620279021569
    },
    "achievement": {
      "_id": 387028092977252,
      "human_code": "AA DEGREE",
      "description": "Associate Degree in Arts satisfying Gen Ed requirements for higher education",
      "credits_available": 4,
      "name": "Associate of Arts Degree",
      "type": "Degree",
      "achievement_type": "Degree"
    },
    "bundleIds": [
      8489347,
      33432186,
      99964903,
      117491737,
      186742988,
      265078206,
      359264226,
      416462341,
      516277473,
      540754377,
      575394182,
      595568130,
      618849082,
      631335297,
      649117162,
      666831520,
      704860485,
      777691543,
      802037171,
      860911325,
      869257144,
      989838243,
      998338412
    ]
  },
  {
    "_id": 351843720888432,
    "results": [
      {
        "_id": 369435906932736
      },
      {
        "_id": 369435906932737
      }
    ],
    "term": "2021 Fall",
    "recipient": {
      "_id": 422212465065999
    },
    "source": {
      "_id": 404620279021568
    },
    "achievement": {
      "_id": 387028092977264,
      "field_of_study": "Education",
      "human_code": "EDU402",
      "description": "Classroom Management. A study of the strategies and techniques for creating and maintaining a positive and productive learning environment, including behavior management and conflict resolution.",
      "credits_available": 3,
      "name": "Classroom Management",
      "type": "Course"
    },
    "bundleIds": [
      33432186,
      441600439
    ]
  },
  {
    "_id": 351843720888431,
    "results": [
      {
        "_id": 369435906932742
      },
      {
        "_id": 369435906932743
      }
    ],
    "term": "2021 Fall",
    "recipient": {
      "_id": 422212465065999
    },
    "source": {
      "_id": 404620279021568
    },
    "achievement": {
      "_id": 387028092977263,
      "field_of_study": "Education",
      "human_code": "EDU401",
      "description": "Curriculum and Instruction. A study of the principles and practices of curriculum development, instruction, and assessment, including alignment with standards and diversity considerations.",
      "credits_available": 3,
      "name": "Curriculum and Instruction",
      "type": "Course"
    },
    "bundleIds": [
      33432186,
      186742988,
      434366823,
      441600439,
      802037171
    ]
  },
...
]
cap10morgan commented 1 year ago

Fix PRs: