ripple / rippled-historical-database

SQL database as a canonical source of historical data
99 stars 68 forks source link

Use consistent column fields, values and column orders in CSV exchanges response #85

Closed donovanhide closed 9 years ago

donovanhide commented 9 years ago

Hi,

the example below shows how the column fields and orders of column fields can vary from request to another (offer_sequence in different positions, autobridge fields in one response, but not the other):

https://gist.github.com/donovanhide/c41f2ef2eb6a0be060d2

Also note that offer_sequence can be either "NaN" or "" for a zero value.

If this data is to be easily consumed, without having lots of variable case handling at the client end, then selecting a fixed column order, with all optional fields represented as null values and consistency in the actual null values is required.

shekenahglory commented 9 years ago

inconsistency fixed with https://github.com/ripple/rippled-historical-database/commit/f5990ab30a1fa8f8441ac0454779fa8b86ee037b - other endpoints may have similar issues. offer_sequence should always be defined, I filled a ticket to get that fixed also

donovanhide commented 9 years ago

Great, will test when deployed!

donovanhide commented 9 years ago

Actually, would offer_sequence be better defaulted to null rather than 0 for when NaN is encountered?

https://github.com/ripple/rippled-historical-database/commit/f5990ab30a1fa8f8441ac0454779fa8b86ee037b#diff-2c3dff3aebfa601935f9eba06cc6f09fL815

That is:

row.offer_sequence = Number(row.offer_sequence || null);

rather than:

row.offer_sequence = Number(row.offer_sequence || 0);
shekenahglory commented 9 years ago

Number(null) and number(0) both resolve to 0 - Number(undefined) resolves to NaN are you saying offer_sequence should be null though, rather than 0 in this case?

donovanhide commented 9 years ago

Well, as far as I know, an Offer can never have a sequence of 0 or 1, because an AccountRoot never has a sequence of 0, and 1 is used by the initial funding payment.

shekenahglory commented 9 years ago

right, thats why I thought 0 would be fine in the meantime, until we get the data fixed, at which point it will be irrelevant because it will always be a nonzero integer.

shekenahglory commented 9 years ago

fixed: https://github.com/ripple/rippled-historical-database/commit/27fe4d77bb01387c77069b896484011cfff348d1