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

feat: Rewriting the value parser #149

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

forsaken628
Copy link
Contributor

  1. the old value parser did not conform to the standard library documentation. driver.Value should be a small number of types, and then the standard library implements conversions to a wider range of types.
  2. provide unknownColumnType for compatibility with newly added types.
  3. passes ttc integration tests.
  4. this PR supports fewer types than the previous one.
  5. need to add support for complex types. That is, add Struct, through the implementation of the Scanner interface, to complete the parsing driver.Value.

@forsaken628
Copy link
Contributor Author

cc @flaneur2020

@@ -77,6 +77,10 @@ func (dc *DatabendConn) BeginTx(
return &databendTx{dc}, nil
}

func (dc *DatabendConn) DataParserOptions() *DataParserOptions {
Copy link
Member

Choose a reason for hiding this comment

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

is this required to be public to users?

@flaneur2020
Copy link
Member

only a nit about whether to expose DataParserOptions in Connection or not, all others are LGTM

@everpcpc @hantmac PTAL

if c.checkNull(s) {
return nil, nil
}
return time.ParseInLocation("2006-01-02 15:04:05.999999", s, time.UTC)
Copy link
Member

Choose a reason for hiding this comment

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

the timezone should be fetched from settings I think.

Copy link
Member

@sundy-li sundy-li Jan 22, 2025

Choose a reason for hiding this comment

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

But we don't have client side timezone, maybe we can keep it let it use UTC now.

I checked bendsql it's also using native date

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants