Fix v2 show if result is empty #30

NRHelmi · 2022-08-17T02:38:52Z

closes #29

cpetitpas · 2022-08-17T15:53:30Z

I has able to use the hnr-fix-show-empty-result branch to build rai-cli by specifying the branch in the rai-cli/go.mod file (rather than v.0.3.1-alpha).

chris@PetitpasServer:~/rai-cli$ git reset --hard
HEAD is now at 564bdf1 upgrade to v2 transactions (#17)
chris@PetitpasServer:~/rai-cli$ ./tidy
rai/rai imports
	github.com/relationalai/rai-sdk-go/rai: missing go.sum entry for module providing package github.com/relationalai/rai-sdk-go/rai (imported by rai/rai); to add:
	go get rai/rai
go: downloading github.com/relationalai/rai-sdk-go v0.3.0-alpha.0.20220817180626-ca64034b6d79
chris@PetitpasServer:~/rai-cli$ go get rai/rai
chris@PetitpasServer:~/rai-cli$ ./tidy
chris@PetitpasServer:~/rai-cli$ ./make

After that ./tidy and ./make the resulting rai does not output the ic violation (stderr).

chris@PetitpasServer:~/Documents/RAI$ raiFix exec cp-testing0816 -e cp-0816 -f tests/icViolationTypeIntFloat.rel
Executing query (cp-testing0816/cp-0816) readonly=false .. Ok (6.0s)
/:abort
()

However, when testing this same built rai version using load_binary, I get:

Executing query (cp-functionalTestDb0817/cp-functionalTestEngine0817) readonly=false .. panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x886c5a]

goroutine 1 [running]:
github.com/relationalai/rai-sdk-go/rai.unmarshal(0xc0000f2280?, {0x964300, 0xc0000bea20})
	/home/chris/rai-cli/vendor/github.com/relationalai/rai-sdk-go/rai/client.go:314 +0x41a
github.com/relationalai/rai-sdk-go/rai.(*Client).request(0x9e4e24?, {0x9e2315, 0x4}, {0x9e5945, 0xd}, 0x7fb65f539108?, 0x404125?, {0x95cde0?, 0xc00021e750?}, {0x964300, ...})
	/home/chris/rai-cli/vendor/github.com/relationalai/rai-sdk-go/rai/client.go:484 +0x19e
github.com/relationalai/rai-sdk-go/rai.(*Client).Post(...)
	/home/chris/rai-cli/vendor/github.com/relationalai/rai-sdk-go/rai/client.go:237
github.com/relationalai/rai-sdk-go/rai.(*Client).ExecuteAsync(0xc000024afa?, {0x7ffd543bc81a?, 0xc0000f2280?}, {0x7ffd543bc835?, 0xc0000bbc08?}, {0xc0000f6360?, 0xc000034680?}, 0x0?, 0x65?)
	/home/chris/rai-cli/vendor/github.com/relationalai/rai-sdk-go/rai/client.go:1237 +0x30d
github.com/relationalai/rai-sdk-go/rai.(*Client).Execute(0xc00021e5d0?, {0x7ffd543bc81a?, 0x6?}, {0x7ffd543bc835?, 0x0?}, {0xc0000f6360?, 0x0?}, 0x0?, 0x0?)
	/home/chris/rai-cli/vendor/github.com/relationalai/rai-sdk-go/rai/client.go:1246 +0x4e
main.execQuery(0xc00021b180?, {0xc00008a320?, 0x1, 0x5})
	/home/chris/rai-cli/rai/cmds.go:578 +0x253
github.com/spf13/cobra.(*Command).execute(0xc00021b180, {0xc00008a2d0, 0x5, 0x5})
	/home/chris/rai-cli/vendor/github.com/spf13/cobra/command.go:876 +0x67b
github.com/spf13/cobra.(*Command).ExecuteC(0xc0000c5680)
	/home/chris/rai-cli/vendor/github.com/spf13/cobra/command.go:990 +0x3b4
github.com/spf13/cobra.(*Command).Execute(...)
	/home/chris/rai-cli/vendor/github.com/spf13/cobra/command.go:918
main.main()
	/home/chris/rai-cli/rai/main.go:315 +0x27a

Other tests seem fine.
I am unsure if this is due to the method I used to build rai-cli using a branch of rai-sdk-go or if it is an issue with the fix here.
The Rel is quite simple:

module config
    def path = "s3://relationalai-rd-language-product-analysis-public2/loadBinaryData/screenshotS3PartitionedCsvData.png"

    
end

def delete[:loadBinaryS3Url] = loadBinaryS3Url
def insert:loadBinaryS3Url = load_binary[config]

Perhaps someone can tell me if my method of building rai-cli using the hnr-fix-show-empty-result` branch is proper? Is there another method?

NRHelmi · 2022-08-17T16:14:12Z

@cpetitpas issue should be fixed can you try again ?

cpetitpas · 2022-08-17T17:02:51Z

After this commit, the load_binary issue no longer occurs. Thanks @NRHelmi . However we're still not getting stderr output for ic violations (possibly others).
For Rel:

def my_data = {3.14}

ic myDataExists(x) = my_data(x) implies Int(x)

....the output is:

Executing query (cp-testing0816/cp-0816) readonly=false .. Ok (5.9s)
/:abort
()

Before this issue the output was:

Executing query (cp-testing0816/cp-0816) readonly=false .. (2.2s)
422 Unprocessable Entity
{"output":[{"rel_key":{"values":["Int64"],"name":"rel","keys":[":catalog",":ic_violation",":range",":start",":character","RelationalAITypes.HashValue"],"type":"RelKey"},"type":"Relation","columns":[["7299174261016051365"],[1]]},{"rel_key":{"values":["Int64"],"name":"rel","keys":[":catalog",":ic_violation",":range",":end",":character","RelationalAITypes.HashValue"],"type":"RelKey"},"type":"Relation","columns":[["7299174261016051365"],[46]]},{"rel_key":{"values":["RelationalAITypes.HashValue",":myDataExists"],"name":"rel","keys":[":catalog",":ic_violation",":name"],"type":"RelKey"},"type":"Relation","columns":[["7299174261016051365"]]},{"rel_key":{"values":["String"],"name":"rel","keys":[":catalog",":ic_violation",":report","RelationalAITypes.HashValue"],"type":"RelKey"},"type":"Relation","columns":[["7299174261016051365"],["4| ic myDataExists(x) = my_data(x) implies Int(x)\n   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n"]]},{"rel_key":{"values":[],"name":"abort","keys":[],"type":"RelKey"},"type":"Relation","columns":[[]]},{"rel_key":{"values":["Float64"],"name":"rel","keys":[":catalog",":ic_violation",":output","RelationalAITypes.HashValue"],"type":"RelKey"},"type":"Relation","columns":[["7299174261016051365"],[3.14]]},{"rel_key":{"values":["Int64"],"name":"rel","keys":[":catalog",":ic_violation",":range",":end",":line","RelationalAITypes.HashValue"],"type":"RelKey"},"type":"Relation","columns":[["7299174261016051365"],[4]]},{"rel_key":{"values":["Int64"],"name":"rel","keys":[":catalog",":ic_violation",":range",":start",":line","RelationalAITypes.HashValue"],"type":"RelKey"},"type":"Relation","columns":[["7299174261016051365"],[4]]},{"rel_key":{"values":["RelationalAITypes.HashValue",":rel-query-action##2975#myDataExists#0"],"name":"rel","keys":[":catalog",":ic_violation",":decl_id"],"type":"RelKey"},"type":"Relation","columns":[["7299174261016051365"]]}],"problems":[{"type":"IntegrityConstraintViolation","sources":[{"rel_key":{"values":[],"name":"_internal_ic_violation","keys":[":rel-query-action##2975#myDataExists#0"],"type":"RelKey"},"type":"ICViolation","source":"4| ic myDataExists(x) = my_data(x) implies Int(x)\n   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n"}]}],"actions":[],"debug_level":0,"aborted":true,"type":"TransactionResult","abort_reason":"integrity constraint violation"}

I run these tests daily and count on this output to ensure IC violation output is proper. It would be great if we can get it to work also.

cpetitpas · 2022-08-17T17:32:06Z

Not surprisingly it seems the stderr issue affects any query which outputs to stderr.

def v = #1
def output = #(v)

.....from https://github.com/RelationalAI/raicode/issues/9707 has the same issue as an ic violation query after this fix.

cpetitpas · 2022-08-17T19:13:07Z

After this commit, the stderr output issue is fixed. The ic violation now returns (including stderr):

//ic violation type int/float
Executing query (cp-functionalTestDb0817/cp-functionalTestEngine0817) readonly=false .. Ok (12.2s)
/:abort
()
/:rel/:catalog/:ic_violation/:decl_id/HashValue/:rel-query-action##434#myDataExists#0
[1.3098243395973143e+19 0]

/:rel/:catalog/:ic_violation/:name/HashValue/:myDataExists
[1.3098243395973143e+19 0]

/:rel/:catalog/:ic_violation/:output/HashValue/Float64
[1.3098243395973143e+19 0], 3.14

/:rel/:catalog/:ic_violation/:range/:end/:character/HashValue/Int64
[1.3098243395973143e+19 0], 46

/:rel/:catalog/:ic_violation/:range/:end/:line/HashValue/Int64
[1.3098243395973143e+19 0], 4

/:rel/:catalog/:ic_violation/:range/:start/:character/HashValue/Int64
[1.3098243395973143e+19 0], 1

/:rel/:catalog/:ic_violation/:range/:start/:line/HashValue/Int64
[1.3098243395973143e+19 0], 4

/:rel/:catalog/:ic_violation/:report/HashValue/String
[1.3098243395973143e+19 0], 4| ic myDataExists(x) = my_data(x) implies Int(x)
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

In running my test suite I still see a few issues:

missing output now incorrect.
For this Rel:

def output = 1, missing, 2

.....the output is now:

Executing query (cp-functionalTestDb0817/cp-functionalTestEngine0817) readonly=false .. Ok (9.9s)
/:output/Int64/Missing/Int64
1, map[], 2

...where previous to the v2 transactions change it was:

Executing query (cp-testing0816/cp-0816) readonly=true .. Ok (6.5s)
# output (Int64*Missing*Int64)
1, <nil>, 2

decimal output incorrect. For this Rel:

def output = decimal[64, 2, 1.23];2.23;3.56

....we now get:

Executing query (cp-functionalTestDb0817/cp-functionalTestEngine0817) readonly=false .. Ok (1.1s)
/:output/FixedPointDecimals.FixedDecimal{Int64, 2}
123

/:output/Float64
2.23
3.56

....where before the v2 transactions change it was:

Executing query (cp-functionalTestDb0816/cp-functionalTestEngine0816) readonly=false .. Ok (1.1s)
# output (FixedPointDecimals.FixedDecimal{Int64, 2})
1.23

# output (Float64)
2.23
3.56

String outputs are missing quotes. This results in output to a file looking a bit strange and potential issues if/when attempting to parse such output. Some examples (before and after):

....and

It would be nice to at least get the decimal and String output issues fixed. I'm not sure what the proper output for missing is but <nil> would seem to make more sense than map[].

NRHelmi · 2022-08-18T05:06:53Z

@cpetitpas I spent sometime looking into the last issues (missing shown as map[], decimal not being correctly printed and String missing quotes) and here is my output:

For decimals I think the issue is expected and decimals require extra processing in v2 protocol
For missing and string issues I "blame" go apache arrow MarshalJson function (not blaming really, MarshalJson is doing its job but it's not fully supporting our specific use cases), I think we need to live with it until we get the result interface implemented. Currently arrow.Array type need to get cast to some arrow.*Array | arrow.Struct types (probably other types) to be able to get its values and this will introduce a big switch statement in the v2 Show function similar to this one https://github.com/apache/arrow/blob/go/v9.0.0/go/arrow/internal/arrjson/arrjson.go#L123-L219 (used by MarshalJson), I believe we will have a similar switch statement when implementing the result interface.
In the mean time I suggest creating an issue for the missing functionalities waiting for the result interface implementation 🙏

@NHDaly for a second eye 👀

cpetitpas · 2022-08-18T13:40:27Z

@NRHelmi - Thank you for your efforts. Makes sense to me.
Would the new ticket go in rai-sdk-go or in rai-cli?

NRHelmi · 2022-08-18T13:46:03Z

@NRHelmi - Thank you for your efforts. Makes sense to me.
Would the new ticket go in rai-sdk-go or in rai-cli?

It makes sense to create the issue ticket as part of rai-sdk-go, thank you :)

cpetitpas · 2022-08-18T13:54:30Z

Thanks. I figured there would be more to it. #31 created.

NHDaly · 2022-08-18T17:43:14Z

👍 cool thanks both!

fix v2 show if result is empty

83769ce

NRHelmi requested review from NHDaly and cpetitpas August 17, 2022 02:38

NRHelmi added 4 commits August 17, 2022 03:43

changelog update

5bb7e21

fix proto metadata issue

0bc2a6d

cleanup

a338d32

fix

47a2d2e

fix for empty outputs

0e5eebf

NRHelmi added 2 commits August 17, 2022 19:03

don't break if arrow table is empty

2f21eb4

fix

ca64034

cpetitpas mentioned this pull request Aug 18, 2022

missing functionalities in result output after update to transactions v2 #31

Open

NRHelmi merged commit ba8d4d3 into main Aug 19, 2022

NRHelmi deleted the hnr-fix-show-empty-result branch August 19, 2022 07:42

cpetitpas mentioned this pull request Aug 19, 2022

ic violation produces huge output of blank lines using --format pretty (the default) #29

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix v2 show if result is empty #30

Fix v2 show if result is empty #30

NRHelmi commented Aug 17, 2022

cpetitpas commented Aug 17, 2022 •

edited

Loading

NRHelmi commented Aug 17, 2022

cpetitpas commented Aug 17, 2022

cpetitpas commented Aug 17, 2022

cpetitpas commented Aug 17, 2022

NRHelmi commented Aug 18, 2022 •

edited

Loading

cpetitpas commented Aug 18, 2022

NRHelmi commented Aug 18, 2022

cpetitpas commented Aug 18, 2022

NHDaly commented Aug 18, 2022

Fix v2 show if result is empty #30

Fix v2 show if result is empty #30

Conversation

NRHelmi commented Aug 17, 2022

cpetitpas commented Aug 17, 2022 • edited Loading

NRHelmi commented Aug 17, 2022

cpetitpas commented Aug 17, 2022

cpetitpas commented Aug 17, 2022

cpetitpas commented Aug 17, 2022

NRHelmi commented Aug 18, 2022 • edited Loading

cpetitpas commented Aug 18, 2022

NRHelmi commented Aug 18, 2022

cpetitpas commented Aug 18, 2022

NHDaly commented Aug 18, 2022

cpetitpas commented Aug 17, 2022 •

edited

Loading

NRHelmi commented Aug 18, 2022 •

edited

Loading