Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arrowBatch getting panic: interface conversion: arrow.Array is *array.Int64, not *array.Timestamp #879

Closed
Yifeng-Sigma opened this issue Aug 6, 2023 · 6 comments
Assignees
Labels
bug Erroneous or unexpected behaviour status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector.

Comments

@Yifeng-Sigma
Copy link
Contributor

in arrowToRecord() when decoding timestampNtzType and timestampLtzType, snowflake can send int64 array, which result in panic: interface conversion: arrow.Array is *array.Int64, not *array.Timestamp because of the following code.
for i, t := range col.(*array.Timestamp).TimestampValues() {

The below fixed things.

		case timestampNtzType:
			tb := array.NewTimestampBuilder(pool, &arrow.TimestampType{Unit: arrow.Nanosecond})
			if col.DataType().ID() == arrow.STRUCT {
				structData := col.(*array.Struct)
				epoch := structData.Field(0).(*array.Int64).Int64Values()
				fraction := structData.Field(1).(*array.Int32).Int32Values()
				for i := 0; i < int(numRows); i++ {
					if !col.IsNull(i) {
						val := time.Unix(epoch[i], int64(fraction[i]))
						tb.Append(arrow.Timestamp(val.UnixNano()))
					} else {
						tb.AppendNull()
					}
				}
			} else if col.DataType().ID() == arrow.INT64 {
				for i, t := range col.(*array.Int64).Int64Values() {
					if !col.IsNull(i) {
						val := time.Unix(0, t*int64(math.Pow10(9-int(srcColumnMeta.Scale)))).UTC()
						tb.Append(arrow.Timestamp(val.UnixNano()))
					} else {
						tb.AppendNull()
					}
				}
			} else {
				for i, t := range col.(*array.Timestamp).TimestampValues() {
					if !col.IsNull(i) {
						val := time.Unix(0, int64(t)*int64(math.Pow10(9-int(srcColumnMeta.Scale)))).UTC()
						tb.Append(arrow.Timestamp(val.UnixNano()))
					} else {
						tb.AppendNull()
					}
				}
			}
			newCol = tb.NewArray()
			defer newCol.Release()
			tb.Release()
		case timestampLtzType:
			tb := array.NewTimestampBuilder(pool, &arrow.TimestampType{Unit: arrow.Nanosecond, TimeZone: loc.String()})
			if col.DataType().ID() == arrow.STRUCT {
				structData := col.(*array.Struct)
				epoch := structData.Field(0).(*array.Int64).Int64Values()
				fraction := structData.Field(1).(*array.Int32).Int32Values()
				for i := 0; i < int(numRows); i++ {
					if !col.IsNull(i) {
						val := time.Unix(epoch[i], int64(fraction[i]))
						tb.Append(arrow.Timestamp(val.UnixNano()))
					} else {
						tb.AppendNull()
					}
				}
			} else if col.DataType().ID() == arrow.INT64 {
				for i, t := range col.(*array.Int64).Int64Values() {
					if !col.IsNull(i) {
						q := t / int64(math.Pow10(int(srcColumnMeta.Scale)))
						r := t % int64(math.Pow10(int(srcColumnMeta.Scale)))
						val := time.Unix(q, r)
						tb.Append(arrow.Timestamp(val.UnixNano()))
					} else {
						tb.AppendNull()
					}
				}
			} else {
				for i, t := range col.(*array.Timestamp).TimestampValues() {
					if !col.IsNull(i) {
						q := int64(t) / int64(math.Pow10(int(srcColumnMeta.Scale)))
						r := int64(t) % int64(math.Pow10(int(srcColumnMeta.Scale)))
						val := time.Unix(q, r)
						tb.Append(arrow.Timestamp(val.UnixNano()))
					} else {
						tb.AppendNull()
					}
				}
			}
			newCol = tb.NewArray()
			defer newCol.Release()
			tb.Release()
@Yifeng-Sigma Yifeng-Sigma added the bug Erroneous or unexpected behaviour label Aug 6, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka self-assigned this Aug 9, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka added status-triage Issue is under initial triage status-information_needed Additional information is required from the reporter labels Aug 9, 2023
@sfc-gh-dszmolka
Copy link
Contributor

hello and thank you for submitting this issue ! do you think you could provide a minimal viable reproduction code snippet which when run, exhibits the error you're encountering?

@Yifeng-Sigma
Copy link
Contributor Author

if you add

{
 			logical:  "timestamp_ltz",
 			physical: "int64",
 			values:   []time.Time{time.Now(), localTime},
 			nrows:    2,
 			rowType:  execResponseRowType{Scale: 9},
 			sc:       arrow.NewSchema([]arrow.Field{{Type: &arrow.Int64Type{}}}, nil),
 			builder:  array.NewInt64Builder(pool),
 			append: func(b array.Builder, vs interface{}) {
 				for _, t := range vs.([]time.Time) {
 					b.(*array.Int64Builder).Append(t.UnixNano())
 				}
 			},
 			compare: func(src interface{}, convertedRec arrow.Record) int {
 				srcvs := src.([]time.Time)
 				for i, t := range convertedRec.Column(0).(*array.Timestamp).TimestampValues() {
 					if srcvs[i].UnixNano() != int64(t) {
 						return i
 					}
 				}
 				return -1
 			},
 		},

to TestArrowRecord() you will be able to see it. Basically server can send int64 as timestamp.

@sfc-gh-dszmolka
Copy link
Contributor

thank you , this looks helpful! we'll take a look.

@sfc-gh-dszmolka sfc-gh-dszmolka added status-in_progress Issue is worked on by the driver team and removed status-triage Issue is under initial triage status-information_needed Additional information is required from the reporter labels Aug 10, 2023
@sfc-gh-dszmolka
Copy link
Contributor

this should also be fixed with PR #918 - thank you for bearing with us while this is being worked on !

@sfc-gh-dszmolka sfc-gh-dszmolka added status-pr_pending_merge A PR is made and is under review bug Erroneous or unexpected behaviour and removed bug Erroneous or unexpected behaviour status-in_progress Issue is worked on by the driver team labels Oct 2, 2023
@sfc-gh-dszmolka
Copy link
Contributor

PR merged and will be part of the October release, expected towards the end of October

@sfc-gh-dszmolka sfc-gh-dszmolka added status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector. and removed status-pr_pending_merge A PR is made and is under review labels Oct 10, 2023
@sfc-gh-dszmolka
Copy link
Contributor

gosnowflake 1.7.0 released with the fix, thank you all for bearing with us !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Erroneous or unexpected behaviour status-fixed_awaiting_release The issue has been fixed, its PR merged, and now awaiting the next release cycle of the connector.
Projects
None yet
Development

No branches or pull requests

3 participants