Skip to content

sql/types: implement NumberType for system numeric types#3442

Merged
angelamayxie merged 2 commits intodolthub:mainfrom
ym0506:issue-10535-system-types-interfaces
Feb 24, 2026
Merged

sql/types: implement NumberType for system numeric types#3442
angelamayxie merged 2 commits intodolthub:mainfrom
ym0506:issue-10535-system-types-interfaces

Conversation

@ym0506
Copy link
Copy Markdown
Contributor

@ym0506 ym0506 commented Feb 23, 2026

Summary

  • Implement sql.NumberType for system numeric system-variable types:
    • SystemBoolType
    • systemIntType
    • systemUintType
    • systemDoubleType
  • Add compile-time interface conformance checks for these types.
  • Add tests to verify:
    • sql.IsNumberType recognizes system numeric types.
    • stats join alignment accepts system numeric types (AlignBuckets path).

Testing

  • go test ./sql/types ./sql/stats
  • go test ./sql/... -run TestDoesNotExist -tags=gms_pure_go

Related

Copy link
Copy Markdown
Contributor

@angelamayxie angelamayxie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the comments I left. Also, I removed the mention in the description to the join issue -- this PR doesn't actually add any relevant testing.

@ym0506
Copy link
Copy Markdown
Contributor Author

ym0506 commented Feb 24, 2026

Thanks for the review — addressed in b0e739b !!

  • Removed the join test from stats since it was not directly relevant.
  • Moved the sql type-interface checks into a dedicated test file:
    sql/types/system_types_test.go

Please let me know if you prefer a different test organization.

Copy link
Copy Markdown
Contributor

@angelamayxie angelamayxie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the contribution :) I'll go ahead and merge it for you

@angelamayxie angelamayxie merged commit e3e3640 into dolthub:main Feb 24, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants