x-pack/metricbeat/module/sql: Problem with handling types properly

[eedugon]

Creating issue with content provided by @shmsr :

Metricbeat SQL module is automatically changing the data types of VARCHAR columns to numeric types when the values are numeric, which causes the losing of the original values in certain cases.

This is easy to reproduce with the following Metricbeat configuration:

- module: sql
  metricsets:
    - query
  period: 10s
  hosts: ["user=<REMOVED> password=<REMOVED> dbname=<REMOVED> sslmode=disable"]
  driver: "postgres"
  sql_query: "select '0054321'::varchar(20) as strCol, 12345 as intCol"
  sql_response_format: table

The previous example will return 2 columns, the first column with value 0054321 (of VARCHAR type) and the second column with 12345 as a number.

But, after running metricbeat, we see the following:

Query:

$ curl -X POST "localhost:9200/metricbeat-8.14.1/_search?pretty" -H 'Content-Type: application/json' -d'
{
  "query": {
    "match": {
      "metricset.name": "query"
    }
  }
}

Response:

"sql" : {
          "driver" : "postgres",
          "query" : "select '0054321'::varchar(20) as strCol, 12345 as intCol",
          "metrics" : {
            "numeric" : {
              "strcol" : 54321,
              "intcol" : 12345
            }
          }
        },

which is incorrect i.e., leading zeroes of VARCHAR are lost. To retain the leading zeroes in strcol should've been ideally string.

The same will happen for example with a VARCHAR value like 5501174335. It ends up being represented as the float 5.501174335E9, which is wrong.

Root cause:

The root problem exists here. If you see []byte and default cases, it handles them as a string and then tries to parse it as float. If it can, it becomes a float else it remains a string.

For better understanding, you can also take a look at the unit tests (test inputs) here.

If you notice in the unit tests, case int, unit, etc. is handled and float64 is expected and not int, unit which is not correct. Similarly more problems exist.

To fix this behavior, change is necessitated to handle these types properly. Also, add cases like:

s := fmt.Sprint(val)
if len(s) > 1 && s[0] == '0' && s[1] != '.' {
	// Preserve string with leading zeros
	return s
}

i.e., any string leading with 0 (not immediately followed by a dot) should be a string. This should handle types like VARCHAR, TEXT, etc.

Also, types like int, uint, etc. should remain as they are not converted to float.

And then, we have to update the code. Here all those types should be handled properly. For example, numeric types in ES should have those types and not just float64 as it has support for more.

[cstegm]

Hi,

I think detecting the type by value is not a good concept. What about other strings like "0.002"? + you never know what ids are out there and how they look like.

You can maybe also handle all data as string and let the user decide what data needs to be converted using processors?
TBH i expected that the raw_data option provides all data without any manipulation or conversion.

[shmsr]

I've again started to taking a look at this:

Current behaviour

• Using a simple test query:

select '0054321'::varchar(20) as strCol, 12345 as intCol

• Current output:

"sql": {
        "query": "select '0054321'::varchar(20) as strCol, 12345 as intCol",
        "metrics": {
            "numeric": {
                "strcol": 54321,
                "intcol": 12345
            }
        },
        "driver": "postgres"
    }

The strcol value is incorrectly converted to 54321 instead of preserving the original value 0054321.

Root Cause

The issue stems from type handling in the module's code. While github.com/lib/pq correctly interprets VARCHAR as a Go string, the current implementation in the SQL module converts all non-special types to float64 if possible, leading to the loss of leading zeros.

See how the github.com/lib/pq (used by SQL module for Postgres) handles the data types: https://pkg.go.dev/github.com/lib/pq#hdr-Data_Types

• integer types smallint, integer, and bigint are returned as int64
• floating-point types real and double precision are returned as float64
• character types char, varchar, and text are returned as string
• temporal types date, time, timetz, timestamp, and timestamptz are returned as time.Time
• the boolean type is returned as bool
• the bytes type is returned as []byte

So varchar is interpreted as a Go string and intCol is interpreted as int64. But how it is being handled? See:

beats/metricbeat/helper/sql/sql.go

Line 169 in ce4a17b

func getValue(pval *interface{}) interface{} {

Except nil, bool, []interface{}, time.Time; every other type is handled like: convert to string and then to float64 is possible or keep it as string but this shouldn't be the case. So, as varchar is interpreted as a Go string, the code converts it to a string and followed by parsing as float64 and hence the loss of leadings 0s are there.

Proposed solution

A fix has been proposed in PR #41607 that properly handles string types. Output after the fix:

"sql": {
        "driver": "postgres",
        "query": "select '0054321'::varchar(20) as strCol, 12345 as intCol",
        "metrics": {
            "string": {
                "strcol": "0054321"
            },
            "numeric": {
                "intcol": 12345
            }
        }
    }

The fix ensures proper type categorization and preservation of string values.

---

---

A helpful temporary workaround is available for those affected by this bug while waiting for the official fix. You can resolve this by modifying the query in your configuration as shown below:

Before:

select '0054321'::varchar(20) as strCol, 12345 as intCol

After:

select 'x' || '0054321'::varchar(20) as strCol, 12345 as intCol

So this appends x to 0054321 i.e., it becomes x0054321. Now when the code tries to convert it from string to float64 it fails and the code keeps it as it is i.e., as a string.

Output will look like this:

 "sql": {
        "driver": "postgres",
        "query": "select 'x' || '0054321'::varchar(20) as strCol, 12345 as intCol",
        "metrics": {
            "numeric": {
                "intcol": 12345
            },
            "string": {
                "strcol": "x0054321"
            }
        }
    }

Notice the value for strcol i.e., x0054321. Now in the ingest pipelines, we can do something like:

{
      "gsub": {
        "field": "sql.metrics.string.strcol",
        "pattern": "^x",
        "replacement": ""
      }
 }

So it drops the x prefix before indexing the document to ES. Read more about the gsub processor here.

But yes, we recommend to wait for the official fix if it is possible.

[lalit-satapathy]

Can this be closed as the PR is merged?

[shmsr]

Can this be closed as the PR is merged?

Yes, I am just waiting for backport 8.x PR to be merged.

[shmsr]

Fix should be available in 8.16.2. FF for 8.16.1 was yesterday. In case the branch is still not cut out for 8.16.1 then the fix could land in next 8.16.1 release but this is unlikely.

The fix will for sure be available in 8.16.2 release.