fix(export): Full CSV/Excel exports respecting SQL_MAX_ROW config

[Vitor-Avila]

SUMMARY

Superset provides two CSV/Excel export options:

• Export to .CSV/Excel: Downloads the chart data (respecting the chart's row limit).
• Export to full .CSV/Excel: This option is behind the ALLOW_FULL_CSV_EXPORT FF (disabled by default) and only available to Table charts. It would download all data from the chart query (up to the SQL_MAX_ROW config).

The full download was not working properly and instead of getting the SQL_MAX_ROW limit, it was getting a default limit of 5000.

This is a follow up to #31241 and #31720, to fix this flow so that the full download respects the SQL_MAX_ROW coinfig.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

No UI changes.

Before this change, Export to full .CSV/Excel results in a file with <= 5k rows. Now, it results in a file with <= SQL_MAX_ROW rows.

TESTING INSTRUCTIONS

Updated one frontend test. For manual testing:

1. Enable the ALLOW_FULL_CSV_EXPORT FF.
2. Set SQL_MAX_ROW to 100000.
3. Create a table chart using the flights dataset with the row limit set to 15000 and add it to a dashboard.
4. Access the dashboard.
5. Click on the chart menu, then Download > Export to .CSV. Validate the file includes 15k rows of data.
6. Click on the chart menu, then Download > Export to full .CSV. Validate the file includes ~55k rows of data.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[korbit-ai]

Review by Korbit AI

Korbit automatically attempts to detect when you fix issues in new commits.

Category	Issue	Status
	Unmemoized Redux Selector ▹ view	🧠 Not in standard

Files scanned

​

File Path	Reviewed
superset-frontend/src/dashboard/components/gridComponents/Chart.jsx	✅

Explore our documentation to understand the languages and file types we support and the files we ignore.

Check out our docs on how you can make Korbit work best for you and your team.

Loving Korbit!? Share us on LinkedIn Reddit and X

[kgabryje]

LGTM

[rusackas]

If I understand correctly, the FF currently lets you grab the entire table regardless of SQL_MAX_ROW. On the one hand, I appreciate this fix, as such a feature could nuke the DB and melt your computer.

On the other hand, this is a departure from behavior some people have come to expect. It's kind of a breaking change, but that's debatable... this almost feels like a bug fix given the above. Nevertheless, I think there should be a note in UPDATING.md about it, and if you want bonus points, maybe we could surface a tooltip or something that propagates SQL_MAX_ROW so the user knows how many rows they can expect.

[rusackas]

After the above, I would almost imagine we could consider turning the FF on full time and deprecating it, which would be nice.

[Vitor-Avila]

@rusackas I think this was already the case before. @kgabryje recently refactored the dashboard loading for performance improvements (#31241), and if you check the superset-frontend/src/dashboard/containers/Chart.jsx file in there maxRows was already set it this way:

...
maxRows: common.conf.SQL_MAX_ROW,
...

I thought about that too (we don't heavily expose this FF in Preset so I wasn't sure) but I think it always had this "precautionary" measure.

[rusackas]

If this doesn't really change the existing behavior, then I'm fine with it as is, merge away! If it does change the behavior, we should probably note it in UPDATING.md so folks are made ware.

[Vitor-Avila]

thanks @rusackas 🙏 I've updated the description to provide additional context about the change here. Let me know if you have any questions!