surrealdb / surrealdb

A scalable, distributed, collaborative, document-graph database, for the realtime web
https://surrealdb.com
Other
27.47k stars 893 forks source link

Bug: Pre-defining edge tables and fields mess them up badly #1302

Open tomsseisums opened 2 years ago

tomsseisums commented 2 years ago

Describe the bug

I was looking into a flow where one could predefine edge table by specifying fields on them but in turn I found out real wonky behavior.

With complex ID's now enabled for edge tables with beta.8, we can have this powerful thing (ran on a completely empty DB):

RELATE somebody:curious->wants->something:cool SET date = time::now(), id = [in, out, date];

That spits out a real nice:

[
    {
        "time": "5.955791ms",
        "status": "OK",
        "result": [
            {
                "date": "2022-10-03T13:51:01.923065Z",
                "id": "wants:[somebody:curious, something:cool, \"2022-10-03T13:51:01.923065Z\"]",
                "in": "somebody:curious",
                "out": "something:cool"
            }
        ]
    }
]

But then, trying to pre-define an edge table, leads to real messed up results (see next section).

Steps to reproduce

When using this definition:

DEFINE TABLE needs SCHEMALESS;
DEFINE FIELD in ON TABLE needs TYPE record(somebody);
DEFINE FIELD out ON TABLE needs TYPE record(something);
DEFINE FIELD date ON TABLE needs VALUE time::now();
DEFINE FIELD id ON TABLE needs VALUE [in, out, date];

1️⃣ Messed up ID

Running this:

RELATE somebody:curious->needs->something:cool;

Produces:

[
    {
        "time": "5.481958ms",
        "status": "OK",
        "result": [
            {
                "date": "2022-10-03T14:14:04.704138Z",
                "id": [
                    null,
                    null,
                    "2022-10-03T14:14:04.704138Z"
                ], /* totally messed up ID */
                "in": "somebody:curious",
                "out": "something:cool"
            }
        ]
    }
]

2️⃣ Other reference type allowed

And running this, does not take into account the type hint on in & out fields:

RELATE something:cool->needs->something:curious;

Producing:

[
    {
        "time": "3.412208ms",
        "status": "OK",
        "result": [
            {
                "date": "2022-10-03T14:23:10.021693Z",
                "id": [
                    null,
                    null,
                    "2022-10-03T14:23:10.021693Z"
                ],
                "in": "something:cool",
                "out": "something:curious"
            }
        ]
    }
]

3️⃣✅ Assertions fail even for correct values

But this is even weirder case, trying to enforce types with assertions:

DEFINE TABLE needs SCHEMALESS;
DEFINE FIELD in ON TABLE needs TYPE record(somebody) ASSERT meta::tb($value) == "somebody";
DEFINE FIELD out ON TABLE needs TYPE record(something) ASSERT meta::tb($value) == "something";
DEFINE FIELD date ON TABLE needs VALUE time::now();
DEFINE FIELD id ON TABLE needs VALUE [in, out, date];

Even the correct RELATE statement now fails:

RELATE somebody:curious->needs->something:cool;

Producing:

[
    {
        "time": "1.6365ms",
        "status": "ERR",
        "detail": "Found NONE for field `in`, with record `needs:aydn3h2976t7i5owd67x`, but field must conform to: meta::tb($value) == \"somebody\""
    }
]

Expected behaviour

1️⃣ Messed up ID's

I'd expect for a valid complex id to be generated akin to this statement:

RELATE somebody:curious->wants->something:cool SET date = time::now(), id = [in, out, date];
-- wants:[somebody:curious, something:cool, \"2022-10-03T13:51:01.923065Z\"]

But maybe a different expression needs to be used for VALUE there?


2️⃣ Other reference type allowed

For standard tables, for a field with TYPE record(x), when creating a record that tries to reference some other type than x, the field is omitted.

But because graphs are somewhat a built in thing with different semantics as well as behavior, where omitting a field in case it doesn't match the given record type doesn't make sense, I'd want to see an error raised by Surreal.


3️⃣✅ Assertions fail even for correct values

It seems that the ordering of operations is messed up or something, but I'd expect that I could run assertions on in, out fields and the $value would be available for assertions.

SurrealDB version

surreal 1.0.0-beta.8 for macos on aarch64

Contact Details

@tomsseisums

Is there an existing issue for this?

Code of Conduct

Dzordzu commented 1 year ago

Notes

WARNING: Up to this date I've never seen surrealdb code, my observations may be wrong. Furthermore this "note" is in a continuous progress - expect changes

I would love to see the separate bug issues, than this large one. If @tobiemh allows me to do that I will split this into smaller parts and add references in this comment

Issues separation

From the given issue one may separate following issues (these three links below are not working, but they refer to the sections of this comment)

record_type_not_forced

Reason

I don't see type check in any form EXCEPT the following one. There is type casting on the level of parsing SurrealQL, but I cannot see anywhere (as of my fast research) validation of the data.

Solution

Maybe SurrealDB should add an option to force certain types without conversion. Ex. create without type creation

CREATE User:MyUser CONTENT { "name":  123  } STRICT

Clauses with such a flag will skip value conversion on the level of the Value and automatically check for type on the within document operations. This functionality can be placed within store method, or just before any call of it. SurrealDB can also use check method for this

autogen_id_in_out_missing

See below

in_out_asserted_null

Both of the issues seem to be caused by the fact that assertions are evaluated on the level of SurrealQL parsing. The values are assigned to them at the later process

See an answer to the related issue

HELP ME!

In order to fix this/find a more in-depth cause of these problems I would like to know:

  1. Is it possible to get fields definitions within doc/check.rs?
  2. How to enable logging from the library doc operations? (I am ether missing calling check() on the document or logging is somehow disabled for this library)