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

Drilling down to detail records from binned results, count doesn't match number of rows returned #15324

Closed
notrom opened this issue Mar 24, 2021 · 1 comment · Fixed by #28465
Assignees
Labels
.Correctness .Frontend Priority:P2 Average run of the mill bug Querying/GUI Query builder catch-all, including simple mode Querying/Parameters & Variables Filter widgets, field filters, variables etc. .Reproduced Issues reproduced in test (usually Cypress) .Team/QueryProcessor :hammer_and_wrench: Type:Bug Product defects
Milestone

Comments

@notrom
Copy link

notrom commented Mar 24, 2021

Describe the bug
Summarise a count by a binned numerical value, then selecting to drill down to "View these X", the number of rows returned doesn't always match the binned count. I think this is due to the edge conditions of the binned range and the filter value being set. The filter values are set to between the bin limits, values that equal the bin limit values are included in both the upper and lower binning drill downs.

Logs
Nothing of interest in the logs

To Reproduce
Steps to reproduce the behavior:

  1. New simple question on Sample Data
  2. "Orders", count by "Quantity", 10 bins
  3. Table visualisation
  4. Bin "10-20", note the count, for me it was 85
  5. Click the count and "View these Orders"
  6. A table of 89 rows is returned

Expected behavior
The number of rows returned by "View these X" should match the bin count

Screenshots
Binned summary:
image
Drilled to 10-20
image
Drilled to 20-30
image

You can see that the highlighted rows are in both drill downs.

Information about your Metabase Installation:

{
  "browser-info": {
    "language": "en-NZ",
    "platform": "Win32",
    "userAgent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.90 Safari/537.36",
    "vendor": "Google Inc."
  },
  "system-info": {
    "file.encoding": "Cp1252",
    "java.runtime.name": "Java(TM) SE Runtime Environment",
    "java.runtime.version": "1.8.0_161-b12",
    "java.vendor": "Oracle Corporation",
    "java.vendor.url": "http://java.oracle.com/",
    "java.version": "1.8.0_161",
    "java.vm.name": "Java HotSpot(TM) Client VM",
    "java.vm.version": "25.161-b12",
    "os.name": "Windows Server 2012 R2",
    "os.version": "6.3",
    "user.language": "en",
    "user.timezone": "Pacific/Auckland"
  },
  "metabase-info": {
    "databases": [
      "sqlserver",
      "postgres",
      "h2"
    ],
    "hosting-env": "unknown",
    "application-database": "postgres",
    "application-database-details": {
      "database": {
        "name": "PostgreSQL",
        "version": "10.5"
      },
      "jdbc-driver": {
        "name": "PostgreSQL JDBC Driver",
        "version": "42.2.18"
      }
    },
    "run-mode": "prod",
    "version": {
      "tag": "v0.38.1",
      "date": "2021-03-03",
      "branch": "release-x.38.x",
      "hash": "79ef63a"
    },
    "settings": {
      "report-timezone": "Pacific/Auckland"
    }
  }
}

Severity
Inconsistency, leads to confusion and assuming that Metabase is getting it wrong, leads to loss of trust in it.

Additional context
Instead of using a between it should probably use the same logic as the binning which I assume is >= the lower limit, and < the upper. Now I write it out I'm not sure if between behaviour is consistent across databases, in SQL Server it's inclusive of both limits.

@notrom notrom added .Needs Triage Type:Bug Product defects labels Mar 24, 2021
@flamber flamber added .Correctness Priority:P2 Average run of the mill bug Querying/GUI Query builder catch-all, including simple mode Querying/Parameters & Variables Filter widgets, field filters, variables etc. and removed .Needs Triage labels Mar 25, 2021
nemanjaglumac added a commit that referenced this issue Mar 29, 2021
@nemanjaglumac nemanjaglumac added this to Backlog in Cypress Testing Mar 29, 2021
nemanjaglumac added a commit that referenced this issue Mar 29, 2021
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label Mar 29, 2021
tsmacdonald pushed a commit that referenced this issue Apr 2, 2021
@ariya ariya self-assigned this Mar 23, 2022
@nemanjaglumac nemanjaglumac removed this from Backlog in Cypress Testing Jan 11, 2023
@camsaul camsaul self-assigned this Feb 20, 2023
@camsaul
Copy link
Member

camsaul commented Feb 20, 2023

We definitely treat between as inclusive (as it is in SQL), but as you've noticed for binning we use >= and < rather than between. So using between for the drill-thru behavior here is definitely the wrong behavior

camsaul added a commit that referenced this issue Feb 20, 2023
camsaul added a commit that referenced this issue Feb 21, 2023
@camsaul camsaul added this to the 0.46 milestone Feb 21, 2023
github-actions bot pushed a commit that referenced this issue Feb 21, 2023
This was referenced Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Correctness .Frontend Priority:P2 Average run of the mill bug Querying/GUI Query builder catch-all, including simple mode Querying/Parameters & Variables Filter widgets, field filters, variables etc. .Reproduced Issues reproduced in test (usually Cypress) .Team/QueryProcessor :hammer_and_wrench: Type:Bug Product defects
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants