Add minimal sql support #144

vr009 · 2022-03-02T22:52:02Z

This patch adds the support of SQL in connector.
Added SQL tests. Updated config.lua for creation the space for
using SQL in tests. Added the check of Tarantool version to skip SQL
tests if Tarantool version < 2.0.0.

Fixes #62

vr009 · 2022-03-03T09:40:17Z

I've faced some issue. I realized that I can't write exapmles of using SQL in connector with covering it by tests in example_test.go due to its way of testing. The problem is that this kind of test uses comments for checking the output (See this). If anybody has an idea how I could avoid testing SQL if it is testing on Tarantool version < 2.0.0, please share it.

DifferentialOrange

Thank you for your patch! It would be really nice to support SQL in connector.

It seems that #62 is now a ticket about SQL EXECUTE support and SQL PREPARE is expected to be supported as the solution of #117 . It is not obvious from issue's name, so I leave this info here for future reviewers.

I don't have an experience with supporting SQL in connectors, so some things seems confusing to me right now. I left some comments, but later I'll try to give this PR a more thorough review.

tarantool_test.go

ligurio · 2022-03-03T16:30:50Z

. If anybody has an idea how I could avoid testing SQL if it is testing on Tarantool version < 2.0.0, please share it.

@vr009, see

go-tarantool/uuid/uuid_test.go

Lines 154 to 165 in bec9f72

    
           isLess, err := test_helpers.IsTarantoolVersionLess(2, 4, 1) 
        
           if err != nil { 
        
           	log.Fatalf("Failed to extract tarantool version: %s", err) 
        
           } 
        
           if isLess { 
        
           	log.Println("Skipping UUID tests...") 
        
           	isUUIDSupported = false 
        
           	return m.Run() 
        
           } else { 
        
           	isUUIDSupported = true 
        
           }

request.go

config.lua

request.go

response.go

tarantool_test.go

request.go

tarantool_test.go

DifferentialOrange

LGTM after minor language fixes.

(To be honest, I'm not confident in my English and it is possible that I added a couple of new mistakes.)

README.md

vr009 · 2022-05-02T10:07:47Z

Have you considered implementing PREPARE as well? Since in real world scenarios most likely you want to prepare & cache and sql statement before executing it

Sure, that is our goal in scope of #117.

request.go

funny-falcon

Почти готово.
Исправь хотя бы FieldByName(key) на Field(i), и будем считать, что ok.

request.go

ligurio

LGTM, see comment about godoc

example_test.go

config.lua

Totktonada · 2022-05-18T21:37:56Z

tarantool_test.go

+	_, err = conn.Replace(spaceNo, []interface{}{uint(1111), "hello", "world"})
+	if err != nil {
+		b.Errorf("No connection available")
+	}


Is it not the same space as used in the SELECT query below (SQL_TEST)?

Yes, the spaceNo was from another one, fixed. Thank you.

Totktonada

I glanced over the changes briefly and have no objections. Feel free to proceed.

This patch adds the support of SQL in connector. Added support of positional and named arguments. Added ExecuteTyped() method for use with custom packing/unpacking for a type. Added all required constants to const.go for encoding SQL in msgpack and decoding response. Added SQL tests. Updated config.lua for creation the space for using SQL in tests. Added the check of Tarantool version to skip SQL tests if tarantool version < 2.0.0. Changed id of the test spaces with id=512 and id=514, cause if using SQL in tarantool there is no ability to set space id explicitly, so it gets created with id=512 by default and conflicts with already existing space with the same id. Added new dependency in go.sum, go.mod for using assert package. Added examples of using SQL queries in example_test.go for compiling the future documentation from sources. Added notes about the version since which Execute() is supported. Closes #62

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch 2 times, most recently from 2b5bde5 to daf1e59 Compare March 3, 2022 09:26

vr009 marked this pull request as ready for review March 3, 2022 09:40

vr009 requested review from DifferentialOrange and ligurio March 3, 2022 09:40

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch from daf1e59 to 5baa19e Compare March 3, 2022 09:41

DifferentialOrange reviewed Mar 3, 2022

View reviewed changes

tarantool_test.go Outdated Show resolved Hide resolved

tarantool_test.go Outdated Show resolved Hide resolved

funny-falcon suggested changes Mar 6, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

request.go Show resolved Hide resolved

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch 3 times, most recently from 45271e2 to f65d459 Compare March 21, 2022 18:09

ligurio requested changes Mar 22, 2022

View reviewed changes

rybakit reviewed Mar 22, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch from f65d459 to 6875af2 Compare March 23, 2022 09:30

vr009 requested review from ligurio, DifferentialOrange and funny-falcon March 23, 2022 09:55

DifferentialOrange reviewed Mar 24, 2022

View reviewed changes

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch 2 times, most recently from 9edb202 to b5da729 Compare March 28, 2022 09:44

DifferentialOrange reviewed Mar 29, 2022

View reviewed changes

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch 2 times, most recently from 0b83fb9 to 154800d Compare March 29, 2022 16:13

vr009 requested a review from DifferentialOrange March 29, 2022 16:29

DifferentialOrange approved these changes Apr 1, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch 2 times, most recently from 58632f6 to 341b74d Compare April 4, 2022 14:12

ligurio requested changes Apr 4, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch 4 times, most recently from 9e7804f to 3a39ba0 Compare May 6, 2022 08:36

funny-falcon reviewed May 11, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

funny-falcon reviewed May 11, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

funny-falcon reviewed May 11, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch from 3a39ba0 to caaa9cd Compare May 11, 2022 09:44

vr009 requested a review from funny-falcon May 11, 2022 10:51

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch 2 times, most recently from 9a2233c to b942c5e Compare May 11, 2022 13:40

funny-falcon reviewed May 15, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

request.go Outdated Show resolved Hide resolved

funny-falcon reviewed May 15, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

ligurio removed their request for review May 16, 2022 09:00

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch from b942c5e to 803a6ce Compare May 16, 2022 18:00

funny-falcon reviewed May 17, 2022

View reviewed changes

request.go Outdated Show resolved Hide resolved

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch 2 times, most recently from 76cd593 to d3bbbb7 Compare May 17, 2022 12:14

vr009 requested a review from funny-falcon May 17, 2022 12:21

funny-falcon approved these changes May 17, 2022

View reviewed changes

ligurio approved these changes May 17, 2022

View reviewed changes

example_test.go Outdated Show resolved Hide resolved

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch from d3bbbb7 to 3b99735 Compare May 17, 2022 14:50

Totktonada reviewed May 18, 2022

View reviewed changes

config.lua Show resolved Hide resolved

Totktonada reviewed May 18, 2022

View reviewed changes

Totktonada approved these changes May 18, 2022

View reviewed changes

vr009 force-pushed the vr009/gh-62-minimal-sql-support branch from 3b99735 to 9d41e1e Compare May 19, 2022 08:24

vr009 merged commit a9e491f into master May 19, 2022

vr009 deleted the vr009/gh-62-minimal-sql-support branch May 19, 2022 10:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add minimal sql support #144

Add minimal sql support #144

vr009 commented Mar 2, 2022 •

edited

Loading

vr009 commented Mar 3, 2022

DifferentialOrange left a comment •

edited

Loading

ligurio commented Mar 3, 2022

DifferentialOrange left a comment

vr009 commented May 2, 2022

funny-falcon left a comment

ligurio left a comment

Totktonada May 18, 2022

vr009 May 19, 2022

Totktonada left a comment

Add minimal sql support #144

Add minimal sql support #144

Conversation

vr009 commented Mar 2, 2022 • edited Loading

vr009 commented Mar 3, 2022

DifferentialOrange left a comment • edited Loading

Choose a reason for hiding this comment

ligurio commented Mar 3, 2022

DifferentialOrange left a comment

Choose a reason for hiding this comment

vr009 commented May 2, 2022

funny-falcon left a comment

Choose a reason for hiding this comment

ligurio left a comment

Choose a reason for hiding this comment

Totktonada May 18, 2022

Choose a reason for hiding this comment

vr009 May 19, 2022

Choose a reason for hiding this comment

Totktonada left a comment

Choose a reason for hiding this comment

vr009 commented Mar 2, 2022 •

edited

Loading

DifferentialOrange left a comment •

edited

Loading