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

Question #42

Open
iz-iznogood opened this issue Aug 26, 2021 · 7 comments
Open

Question #42

iz-iznogood opened this issue Aug 26, 2021 · 7 comments

Comments

@iz-iznogood
Copy link
Contributor

Hello,

is there a way to use an SQL Server timestamp (rowversion) column with breeze.js and breeze-sequelize ?
If you can suggest a path, I can try to implement it if possible
I mapped it to a binary column and it returns a buffer object but it is not very easy to do a query with it

Thanks

@steveschmitt
Copy link
Member

Life, the universe, and everything!

(because this is Question #42)

@steveschmitt
Copy link
Member

Seems like a good idea. How did you map it? What type do you think it should have?

@iz-iznogood
Copy link
Contributor Author

Life, the universe, and everything!

(because this is Question #42)

Well they solved this one, so lets fix the RowVersion and claim some fame for us !!!

Its been a long time since I touched this but looking in my code, I see that I mapped it to breeze.DataType.Binary and this works fine but the result is something like this

RowVersion: {
"type": "Buffer",
"data": [
0,
0,
0,
0,
0,
0,
27,
159
]
}

RowVersion is the property name in the entity
This is fine but I do not know how to write a query using this kind of object. Even on Sequelize I do not know how to do this
The objective is to select all records with RowVersion > @p where p is a rowversion value send by the client

After digging into the code I found this little piece of code (i am sure it is not mine) but i do not know if there is a better way to do this. Can you give some insights please ?

// This function can be user to compare RowVersion in predicates
// ex: new breeze.Predicate('RowVersion' , 'gt', bufferToHex(rowVersionData));
// where rowVersionData is the RowVersion.data property.
// bufferToHex generates a sql compatible string for comparison ex: 0x0000000000001364
static bufferToHex(buffer) {

    const hexChar = ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "A", "B", "C", "D", "E", "F"];

    const byteToHex = b => {
        // eslint-disable-next-line no-bitwise
        return hexChar[(b >> 4) & 0x0f] + hexChar[b & 0x0f];
    }

    return '0x' + buffer.reduce((string, byte) => {
        return string + byteToHex(byte);
    }, "");

}

@steveschmitt
Copy link
Member

I'd rather not have to do that byte array manipulation.

This StackOverflow answer says that RowVersion can be converted to BigInt. Do you suppose there's a way to make Sequelize do it for us?

@iz-iznogood
Copy link
Contributor Author

iz-iznogood commented Aug 27, 2021

We can do this on the sequelize side, let me check, but the RowVersion is big endian so not the kind of integer we normally use.

Not sure how it will work aa a parameter, we should avoid the conversions for performance issues on the DB.

Will get back soon, thanks

@iz-iznogood
Copy link
Contributor Author

iz-iznogood commented Aug 31, 2021

Hello again and sorry for the delayed response.

I checked sequelize and breeze and I still believe that the above function is better that using BigInt because it generates a query like "SELECT col1, col2 ... FROM table WHERE RowVersion > 0x00000000000009B6".
This query is working perfectly because it is the direct binary representation of the buffer

So my proposal is that, unless we do a change on all sides (breeze and sequelize) to support Buffers in predicates I think this works quite well.

Maybe I can look for a better utility method but at the moment I am 100% covered for what I need so unless you have a better approach we can close this

Something like this works well

const buf = Buffer.from(RowVersion.data); const s = buf.toString("hex")

Thanks

@iz-iznogood
Copy link
Contributor Author

Hello @steveschmitt ,

I am still working on this as part of a larger project I have and I found an issue regarding model create and concurrency fields.
So can you please review and if possible merge it upstream so that I can move forward ?

As I mentioned before, for my project the select does what I need, but I try to see the whole picture and fix what-ever issues I find and might be acceptable by you for merging. I will have probably a few more patches coming your way but I need this to move forward because as it is now, it breaks early.

Hope you can merge it soon

Huge thanks

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

No branches or pull requests

2 participants