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

Return rows with column names as HashMap Text Value #5

Open
vagifverdi opened this issue Mar 31, 2018 · 14 comments
Open

Return rows with column names as HashMap Text Value #5

vagifverdi opened this issue Mar 31, 2018 · 14 comments
Labels
enhancement New feature or request

Comments

@vagifverdi
Copy link

It would be good to be able to create a Map Text Value instead of a list or tuple of values as a row.

For large queries with many columns it is quite cumbersome to use positioning instead of column name.

Also for a drop in replacement of old HDBC-odbc code, column names are needed, since I exclusively use Maps instead of lists as rows.

@chrisdone chrisdone added the enhancement New feature or request label Mar 31, 2018
@chrisdone
Copy link
Contributor

This is doable with the current interface. 👍

@vagifverdi
Copy link
Author

I'm trying to do it myself. But I'm stumped with C to haskell string conversions.

I added columnName to Column record:

data Column = Column
  { columnType :: !SQLSMALLINT
  , columnSize :: !SQLULEN
  , columnDigits :: !SQLSMALLINT
  , columnNull :: !SQLSMALLINT
  , columnName :: !SQLWCHAR
  } deriving (Show)

In describeColumn i'm trying to get the name of the column:

                                  colname <- peek (coerce namep)
                                  evaluate
                                    Column
                                    { columnType = typ
                                    , columnSize = size
                                    , columnDigits = digits
                                    , columnNull = isnull
                                    , columnName = colname

But that gives me SQLWCHAR. How do i convert it to haskell Text or String?

@chrisdone
Copy link
Contributor

chrisdone commented Mar 31, 2018

T.fromPtr can be used like this to give you a Text from a wchar_t* (SQLWCHAR).

@chrisdone
Copy link
Contributor

Be aware that T.useAsPtr, used here, frees the allocated memory after it's finished, so you'll want to do the T.fromPtr (which copies) inside that call.

@vagifverdi
Copy link
Author

ok, a little progress. I tried
colname <- T.fromPtr namep (fromIntegral (div namelen 2))

And it produced a string that starts with a field name bu then continues with a large string of prefilled zeroes like this:

"min_time\NUL00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"

Notice that there's a null terminator there.

How would I deal with tat?

@chrisdone
Copy link
Contributor

chrisdone commented Mar 31, 2018

After that, I think it's fine if we make a breaking API change: fetchIterator, fetchStatementRows, query and stream all produce rows of HashMap Text Value instead of [Maybe Value]. The fromRow class in Conversion will have to change. Perhaps you can rename fromRow to fromRowList and add another method fromRowMap :: HashMap Text Value -> Either String r, and have fromRowMap implemented in terms of the already existing method, fromRowMap = fromRowList . M.elems. And then you just add an instance for FromValue v => FromRow (HashMap Text v). The *.SQLServer module need not change.

HashMap is much more memory efficient than Map: https://github.com/haskell-perf/dictionaries#pure-maps-fromlist-space-use (And faster lookup: https://github.com/haskell-perf/dictionaries#lookupbytestring-monotonic)

@chrisdone
Copy link
Contributor

colname <- T.fromPtr namep (fromIntegral (div namelen 2))

namelen comes from here, so it should be 2000. Instead, you want to use the length given by the ODBC driver, which is put into namelenp. You can peek that pointer to get the actual length of the column.

@vagifverdi
Copy link
Author

Success! Thank you very much for helping me.

  colnamelen  <- peek namelenp
  colname <- T.fromPtr namep (fromIntegral colnamelen)

For my case I simply cloned query and fetchStatementRows into queryMaps and fetchStatementMaps

Perhaps as the next step I could tackle unifying them all as you suggested.

Also if we are open to API redesign at this early stage what do you think about dropping your Value and adopting SqlValue from HDBC package?
Otherwise I have to create a compatibility layer converting from Value to SqlValue in order to make this a drop in replacement for hdbc-odbc.

@chrisdone
Copy link
Contributor

chrisdone commented Mar 31, 2018

Perhaps as the next step I could tackle unifying them all as you suggested.

👍

Also if we are open to API redesign at this early stage what do you think about dropping your Value and adopting SqlValue from HDBC package?

I'm not attached to Value, it's just shorter. SqlValue is fine.

Otherwise I have to create a compatibility layer converting from Value to SqlValue in order to make this a drop in replacement for hdbc-odbc.

What about the constructors of Value - are they largely the same? Is that important?

@vagifverdi
Copy link
Author

Value and SqlValue have almost the same structure. SqlValue has SqlNull that allows to drop Maybe and has automatic way to convert to maybes if needed.

https://hackage.haskell.org/package/HDBC-2.4.0.2/docs/Database-HDBC-SqlValue.html

But yes, there are more constructors for SqlValue than what you have.

@vagifverdi
Copy link
Author

Also Value clashes with Aeson.

@vagifverdi
Copy link
Author

Hmm, we do not need to provide an implementation for every Constructor in the SqlValue. Only the ones that we are interested in.

@chrisdone
Copy link
Contributor

OK, good, then we can deal with Value in a separate discussion. So we just have the HashMap part. I won't be able to work on this today, but perhaps tomorrow.

@vagifverdi
Copy link
Author

Yeah, HDBC still uses String instead of Text in SqlValue.
Perhaps I should raise the issue with them, but I doubt this will be fixed. Probably too much legacy code everywhere.

@chrisdone chrisdone changed the title Column names are lost Return rows with column names as HashMap Text Value Apr 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants