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

Problem with boolean and decimal values #89

Closed
moander opened this issue Apr 17, 2018 · 7 comments
Closed

Problem with boolean and decimal values #89

moander opened this issue Apr 17, 2018 · 7 comments
Assignees
Labels
api: bigtable Issues related to the googleapis/nodejs-bigtable API. 🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@moander
Copy link

moander commented Apr 17, 2018

I'm seeing this result. bool and bool_t should be true, bool_f should be false, and dec should be 1.23

{
  "id": "gwashington",
  "data": {
    "fam1": {
      "bin": [
        {
          "value": "abc",
          "labels": [],
          "timestamp": "1523953458013000"
        }
      ],
      "bool": [
        {
          "value": "",
          "labels": [],
          "timestamp": "1523953458013000"
        }
      ],
      "bool_f": [
        {
          "value": "",
          "labels": [],
          "timestamp": "1523953458013000"
        }
      ],
      "bool_t": [
        {
          "value": "",
          "labels": [],
          "timestamp": "1523953458013000"
        }
      ],
      "dec": [
        {
          "value": 1,
          "labels": [],
          "timestamp": "1523953458013000"
        }
      ],
      "jadams": [
        {
          "value": 1,
          "labels": [],
          "timestamp": "1523953458013000"
        }
      ],
      "str": [
        {
          "value": "hello",
          "labels": [],
          "timestamp": "1523953458013000"
        }
      ]
    }
  }
}

Environment details

  • OS: macos 10.13.3
  • Node.js version: 8.10.0
  • npm version: 5.6.0
  • yarn version: 1.3.2
  • google-cloud-node version: master branch (0.13)

Steps to reproduce

    const entries = [
        {
          method: 'insert',
          key: 'gwashington',
          data: {
            fam1: {
                jadams: 1,
                bool: true,
                bool_t: true,
                bool_f: false,
                dec: 1.23,
                str: "hello",
                bin: Buffer.from('abc'),
            }
          }
        }
      ];

    await table.mutate(entries);
@stephenplusplus stephenplusplus added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Apr 17, 2018
@stephenplusplus
Copy link
Contributor

I was able to reproduce.

cc @kolodny

@kolodny
Copy link
Contributor

kolodny commented Apr 17, 2018

I was able to repro too. This bug doesn't seem new though since I was able to repro as far back as v0.10.2. It appears the only difference is that having bools in the data used to throw TypeError: Illegal buffer and after #35 it just puts it bools in as empty strings.

What is the current state with booleans in Bigtable? Does it get encoded in some special way that we can encode/decode when writing/reading that value?

cc @sduskis

@stephenplusplus
Copy link
Contributor

Here's the gapic ReadRowsResponse for inserting a decimal, 1.23:

{
  "chunks": [
    {
      "labels": [],
      "rowKey": {
        "type": "Buffer",
        "data": [
          103,
          119,
          97,
          115,
          104,
          105,
          110,
          103,
          116,
          111,
          110
        ]
      },
      "familyName": {
        "value": "follows"
      },
      "qualifier": {
        "value": {
          "type": "Buffer",
          "data": [
            106,
            97,
            100,
            97,
            109,
            115
          ]
        }
      },
      "timestampMicros": "1524072042604000",
      "value": {
        "type": "Buffer",
        "data": [
          0,
          0,
          0,
          0,
          0,
          0,
          0,
          1
        ]
      },
      "valueSize": 0,
      "commitRow": true,
      "rowStatus": "commitRow"
    }
  ],
  "lastScannedRowKey": []
}

And for true:

{
  "chunks": [
    {
      "labels": [],
      "rowKey": {
        "type": "Buffer",
        "data": [
          103,
          119,
          97,
          115,
          104,
          105,
          110,
          103,
          116,
          111,
          110
        ]
      },
      "familyName": {
        "value": "follows"
      },
      "qualifier": {
        "value": {
          "type": "Buffer",
          "data": [
            106,
            97,
            100,
            97,
            109,
            115
          ]
        }
      },
      "timestampMicros": "1524071811968000",
      "value": [],
      "valueSize": 0,
      "commitRow": true,
      "rowStatus": "commitRow"
    }
  ],
  "lastScannedRowKey": []
}

At this point, it seems impossible to derive 1.23 from value: [0, 0, 0, 0, 0, 0, 0, 1] and true from value: [].

@kolodny
Copy link
Contributor

kolodny commented Apr 20, 2018

Bigtable stores all it's fields in binary with no encoding information. The typical way to put data inside bigtable or extract it out is by manually doing the encoding/decoding. The library currently helps out by converting numbers to Buffers otherwise it just passes whatever the value is to Buffer.from and hopes for the best. Ideally every field should be encoded before being passed to table.mutate.

In the future we may consider allowing passing some Mutation like API to simplify the encoding/decoding actions to and from the table

@kolodny kolodny self-assigned this Apr 20, 2018
@kolodny
Copy link
Contributor

kolodny commented Apr 20, 2018

One thing to note based on the above, any data previously entered in that fashion should be considered bad and should be deleted

I'll close this issue for now, feel free to reopen if you have any followup questions or issues

@kolodny kolodny closed this as completed Apr 20, 2018
@ghost ghost removed the priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. label Apr 20, 2018
@moander
Copy link
Author

moander commented Apr 22, 2018

Thanks. I would consider throwing on everything except string buffer and null/undefined instead of potentially storing bad values. After testing to see that string/int works you may move on thinking that also bool and decimal works (like I almost did).

@kolodny
Copy link
Contributor

kolodny commented Apr 24, 2018

Thanks @moander, I opened #102 to get a discussion about that point started.

@google-cloud-label-sync google-cloud-label-sync bot added the api: bigtable Issues related to the googleapis/nodejs-bigtable API. label Jan 31, 2020
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/nodejs-bigtable API. 🚨 This issue needs some love. triage me I really want to be triaged. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

4 participants