Skip to content

Commit

Permalink
Added a new detector for catching unsupported COPY command structure
Browse files Browse the repository at this point in the history
  • Loading branch information
ShivanshGahlot committed Dec 19, 2024
1 parent 75ab89e commit 48f6f7c
Show file tree
Hide file tree
Showing 14 changed files with 148 additions and 15 deletions.
2 changes: 1 addition & 1 deletion yb-voyager/cmd/analyzeSchema.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
"strings"
"text/template"

pg_query "github.com/pganalyze/pg_query_go/v5"
pg_query "github.com/pganalyze/pg_query_go/v6"
"github.com/samber/lo"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand Down
2 changes: 1 addition & 1 deletion yb-voyager/cmd/exportSchema.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"strings"

"github.com/fatih/color"
pg_query "github.com/pganalyze/pg_query_go/v5"
pg_query "github.com/pganalyze/pg_query_go/v6"
"github.com/samber/lo"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand Down
2 changes: 1 addition & 1 deletion yb-voyager/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ require (
github.com/mcuadros/go-version v0.0.0-20190830083331-035f6764e8d2
github.com/mitchellh/go-ps v1.0.0
github.com/nightlyone/lockfile v1.0.0
github.com/pganalyze/pg_query_go/v5 v5.1.0
github.com/pganalyze/pg_query_go/v6 v6.0.0
github.com/samber/lo v1.38.1
github.com/sirupsen/logrus v1.9.3
github.com/spf13/cobra v1.6.1
Expand Down
4 changes: 2 additions & 2 deletions yb-voyager/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1709,8 +1709,8 @@ github.com/pelletier/go-toml/v2 v2.0.5 h1:ipoSadvV8oGUjnUbMub59IDPPwfxF694nG/jwb
github.com/pelletier/go-toml/v2 v2.0.5/go.mod h1:OMHamSCAODeSsVrwwvcJOaoN0LIUIaFVNZzmWyNfXas=
github.com/performancecopilot/speed/v4 v4.0.0/go.mod h1:qxrSyuDGrTOWfV+uKRFhfxw6h/4HXRGUiZiufxo49BM=
github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
github.com/pganalyze/pg_query_go/v5 v5.1.0 h1:MlxQqHZnvA3cbRQYyIrjxEjzo560P6MyTgtlaf3pmXg=
github.com/pganalyze/pg_query_go/v5 v5.1.0/go.mod h1:FsglvxidZsVN+Ltw3Ai6nTgPVcK2BPukH3jCDEqc1Ug=
github.com/pganalyze/pg_query_go/v6 v6.0.0 h1:in6RkR/apfqlAtvqgDxd4Y4o87a5Pr8fkKDB4DrDo2c=
github.com/pganalyze/pg_query_go/v6 v6.0.0/go.mod h1:nvTHIuoud6e1SfrUaFwHqT0i4b5Nr+1rPWVds3B5+50=
github.com/pierrec/lz4 v1.0.2-0.20190131084431-473cd7ce01a1/go.mod h1:3/3N9NVKO0jef7pBehbT1qWhCMrIgbYNnFAZCqQ5LRc=
github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4/go.mod h1:N6UoU20jOqggOuDwUaBQpluzLNDqif3kq9z2wpdYEfQ=
github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU=
Expand Down
8 changes: 5 additions & 3 deletions yb-voyager/src/query/queryissue/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ package queryissue

// Types
const (
ADVISORY_LOCKS = "ADVISORY_LOCKS"
SYSTEM_COLUMNS = "SYSTEM_COLUMNS"
XML_FUNCTIONS = "XML_FUNCTIONS"
ADVISORY_LOCKS = "ADVISORY_LOCKS"
SYSTEM_COLUMNS = "SYSTEM_COLUMNS"
XML_FUNCTIONS = "XML_FUNCTIONS"
COPY_FROM_WHERE = "COPY_FROM_WHERE"
COPY_ON_ERROR = "COPY_ON_ERROR"

REFERENCED_TYPE_DECLARATION = "REFERENCED_TYPE_DECLARATION"
STORED_GENERATED_COLUMNS = "STORED_GENERATED_COLUMNS"
Expand Down
58 changes: 55 additions & 3 deletions yb-voyager/src/query/queryissue/detectors.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ import (
)

const (
ADVISORY_LOCKS_NAME = "Advisory Locks"
SYSTEM_COLUMNS_NAME = "System Columns"
XML_FUNCTIONS_NAME = "XML Functions"
ADVISORY_LOCKS_NAME = "Advisory Locks"
SYSTEM_COLUMNS_NAME = "System Columns"
XML_FUNCTIONS_NAME = "XML Functions"
COPY_FROM_WHERE_NAME = "COPY FROM ... WHERE"
COPY_ON_ERROR_NAME = "COPY ... ON_ERROR"
)

// To Add a new unsupported query construct implement this interface for all possible nodes for that construct
Expand Down Expand Up @@ -141,3 +143,53 @@ func (d *RangeTableFuncDetector) Detect(msg protoreflect.Message) ([]string, err
}
return nil, nil
}

type CopyCommandUnsupportedConstructsDetector struct{}

func NewCopyCommandUnsupportedConstructsDetector() *CopyCommandUnsupportedConstructsDetector {
return &CopyCommandUnsupportedConstructsDetector{}
}

// Detect if COPY command uses unsupported syntax i.e. COPY FROM ... WHERE and COPY... ON_ERROR
func (d *CopyCommandUnsupportedConstructsDetector) Detect(msg protoreflect.Message) ([]string, error) {
unsupportedConstructs := []string{}

// Check if the message is a COPY statement
if msg.Descriptor().FullName() != queryparser.PG_QUERY_COPYSTSMT_NODE {
return unsupportedConstructs, nil // Not a COPY statement, nothing to detect
}

// Check for COPY FROM ... WHERE clause
isFromField := msg.Descriptor().Fields().ByName("is_from")
whereField := msg.Descriptor().Fields().ByName("where_clause")
if isFromField != nil && msg.Has(isFromField) {
isFrom := msg.Get(isFromField).Bool()
if isFrom && whereField != nil && msg.Has(whereField) {
unsupportedConstructs = append(unsupportedConstructs, COPY_FROM_WHERE_NAME)
}
}

// Check for COPY ... ON_ERROR clause
optionsField := msg.Descriptor().Fields().ByName("options")
if optionsField != nil && msg.Has(optionsField) {
optionsList := msg.Get(optionsField).List()
for i := 0; i < optionsList.Len(); i++ {
option := optionsList.Get(i).Message()

// Check for nested def_elem field
defElemField := option.Descriptor().Fields().ByName("def_elem")
if defElemField != nil && option.Has(defElemField) {
defElem := option.Get(defElemField).Message()
defNameField := defElem.Descriptor().Fields().ByName("defname")
if defNameField != nil && defElem.Has(defNameField) {
defName := defElem.Get(defNameField).String()
if defName == "on_error" {
unsupportedConstructs = append(unsupportedConstructs, COPY_ON_ERROR_NAME)
}
}
}
}
}

return unsupportedConstructs, nil
}
47 changes: 47 additions & 0 deletions yb-voyager/src/query/queryissue/detectors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,53 @@ import (
"github.com/yugabyte/yb-voyager/yb-voyager/src/query/queryparser"
)

func TestCopyCommandUnsupportedConstructsDetector(t *testing.T) {
copyCommandSqlsMap := map[string][]string{
// Valid COPY commands without WHERE or ON_ERROR
`COPY my_table FROM '/path/to/data.csv' WITH (FORMAT csv);`: {},
`COPY my_table FROM '/path/to/data.csv' WITH (FORMAT text);`: {},
`COPY my_table FROM '/path/to/data.csv';`: {},
`COPY my_table FROM '/path/to/data.csv' WITH (DELIMITER ',');`: {},
`COPY my_table(col1, col2) FROM '/path/to/data.csv' WITH (FORMAT csv, HEADER true);`: {},

// COPY commands with WHERE clause
`COPY my_table FROM '/path/to/data.csv' WHERE col1 > 100;`: {COPY_FROM_WHERE_NAME},
`COPY my_table(col1, col2) FROM '/path/to/data.csv' WHERE col2 = 'test';`: {COPY_FROM_WHERE_NAME},
`COPY my_table FROM '/path/to/data.csv' WHERE TRUE;`: {COPY_FROM_WHERE_NAME},

// COPY commands with ON_ERROR clause
`COPY table_name (name, age) FROM '/path/to/data.csv' WITH (FORMAT csv, HEADER true, ON_ERROR IGNORE);`: {COPY_ON_ERROR_NAME},
`COPY table_name (name, age) FROM '/path/to/data.csv' WITH (FORMAT csv, HEADER true, ON_ERROR STOP);`: {COPY_ON_ERROR_NAME},

// COPY commands with both ON_ERROR and WHERE clause
`COPY table_name (name, age) FROM '/path/to/data.csv' WITH (FORMAT csv, HEADER true, ON_ERROR IGNORE) WHERE age > 18;`: {COPY_FROM_WHERE_NAME, COPY_ON_ERROR_NAME},
`COPY table_name (name, age) FROM '/path/to/data.csv' WITH (FORMAT csv, HEADER true, ON_ERROR STOP) WHERE name = 'Alice';`: {COPY_FROM_WHERE_NAME, COPY_ON_ERROR_NAME},
}

detector := NewCopyCommandUnsupportedConstructsDetector()
for sql, expectedConstructs := range copyCommandSqlsMap {
parseResult, err := queryparser.Parse(sql)
assert.NoError(t, err, "Failed to parse SQL: %s", sql)

visited := make(map[protoreflect.Message]bool)
unsupportedConstructs := []string{}

processor := func(msg protoreflect.Message) error {
constructs, err := detector.Detect(msg)
if err != nil {
return err
}
unsupportedConstructs = append(unsupportedConstructs, constructs...)
return nil
}

parseTreeMsg := queryparser.GetProtoMessageFromParseTree(parseResult)
err = queryparser.TraverseParseTree(parseTreeMsg, visited, processor)
assert.NoError(t, err)
assert.ElementsMatch(t, expectedConstructs, unsupportedConstructs, "Unsupported Constructs not detected in SQL: %s", sql)
}
}

func TestFuncCallDetector(t *testing.T) {
advisoryLockSqls := []string{
`SELECT pg_advisory_lock(100), COUNT(*) FROM cars;`,
Expand Down
26 changes: 26 additions & 0 deletions yb-voyager/src/query/queryissue/issues_dml.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,29 @@ var xmlFunctionsIssue = issue.Issue{
func NewXmlFunctionsIssue(objectType string, objectName string, sqlStatement string) QueryIssue {
return newQueryIssue(xmlFunctionsIssue, objectType, objectName, sqlStatement, map[string]interface{}{})
}

var copyFromWhereIssue = issue.Issue{
Type: COPY_FROM_WHERE,
TypeName: "COPY FROM ... WHERE",
TypeDescription: "",
Suggestion: "",
GH: "",
DocsLink: "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#copy-from-where-is-not-yet-supported",
}

func NewCopyFromWhereIssue(objectType string, objectName string, sqlStatement string) QueryIssue {
return newQueryIssue(copyFromWhereIssue, objectType, objectName, sqlStatement, map[string]interface{}{})
}

var copyOnErrorIssue = issue.Issue{
Type: COPY_ON_ERROR,
TypeName: "COPY ... ON_ERROR",
TypeDescription: "",
Suggestion: "",
GH: "",
DocsLink: "https://docs.yugabyte.com/preview/yugabyte-voyager/known-issues/postgresql/#copy-on-error-is-not-yet-supported",
}

func NewCopyOnErrorIssue(objectType string, objectName string, sqlStatement string) QueryIssue {
return newQueryIssue(copyOnErrorIssue, objectType, objectName, sqlStatement, map[string]interface{}{})
}
5 changes: 5 additions & 0 deletions yb-voyager/src/query/queryissue/parser_issue_detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ func (p *ParserIssueDetector) getDMLIssues(query string) ([]QueryIssue, error) {
NewColumnRefDetector(),
NewXmlExprDetector(),
NewRangeTableFuncDetector(),
NewCopyCommandUnsupportedConstructsDetector(),
}

processor := func(msg protoreflect.Message) error {
Expand Down Expand Up @@ -382,6 +383,10 @@ func (p *ParserIssueDetector) getDMLIssues(query string) ([]QueryIssue, error) {
result = append(result, NewSystemColumnsIssue(DML_QUERY_OBJECT_TYPE, "", query))
case XML_FUNCTIONS_NAME:
result = append(result, NewXmlFunctionsIssue(DML_QUERY_OBJECT_TYPE, "", query))
case COPY_FROM_WHERE_NAME:
result = append(result, NewCopyFromWhereIssue(DML_QUERY_OBJECT_TYPE, "", query))
case COPY_ON_ERROR_NAME:
result = append(result, NewCopyOnErrorIssue(DML_QUERY_OBJECT_TYPE, "", query))
}
}
return result, nil
Expand Down
2 changes: 1 addition & 1 deletion yb-voyager/src/query/queryparser/ddl_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"slices"
"strings"

pg_query "github.com/pganalyze/pg_query_go/v5"
pg_query "github.com/pganalyze/pg_query_go/v6"
"github.com/samber/lo"
)

Expand Down
2 changes: 1 addition & 1 deletion yb-voyager/src/query/queryparser/helpers_protomsg.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ package queryparser
import (
"strings"

pg_query "github.com/pganalyze/pg_query_go/v5"
pg_query "github.com/pganalyze/pg_query_go/v6"
log "github.com/sirupsen/logrus"
"google.golang.org/protobuf/reflect/protoreflect"
)
Expand Down
2 changes: 1 addition & 1 deletion yb-voyager/src/query/queryparser/helpers_struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import (
"fmt"
"strings"

pg_query "github.com/pganalyze/pg_query_go/v5"
pg_query "github.com/pganalyze/pg_query_go/v6"
"github.com/samber/lo"
)

Expand Down
2 changes: 1 addition & 1 deletion yb-voyager/src/query/queryparser/query_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ package queryparser
import (
"fmt"

pg_query "github.com/pganalyze/pg_query_go/v5"
pg_query "github.com/pganalyze/pg_query_go/v6"
log "github.com/sirupsen/logrus"
)

Expand Down
1 change: 1 addition & 0 deletions yb-voyager/src/query/queryparser/traversal_proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const (
PG_QUERY_INSERTSTMT_NODE = "pg_query.InsertStmt"
PG_QUERY_UPDATESTMT_NODE = "pg_query.UpdateStmt"
PG_QUERY_DELETESTMT_NODE = "pg_query.DeleteStmt"
PG_QUERY_COPYSTSMT_NODE = "pg_query.CopyStmt"
)

// function type for processing nodes during traversal
Expand Down

0 comments on commit 48f6f7c

Please sign in to comment.