Details

Reviewers

ashoat
tomek

Commits

rCOMM04c011143b13: [keyserver] Increase `social_proof` column of `cookies` table from `varchar…

Summary

Was experiencing issues populating the social_proof column of the cookies table because the string I was attempting to store exceeded the "width" of the column:

This diff just increases the size to ensure that the message fits.

Depends on D6134

Test Plan

Ensure migration ran successfully:

Didn't test create-db, but confident things will work as expected.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

atul created this revision.Dec 31 2022, 4:58 AM

atul published this revision for review.Dec 31 2022, 4:58 AM

atul edited the summary of this revision. (Show Details)

atul edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B15010: Diff 20487.Dec 31 2022, 5:11 AM

atul added a child revision: D6136: [keyserver] Store `SIWESocialProof` in `social_proof` column of `cookies` table.Dec 31 2022, 5:41 AM

1024->4096

atul retitled this revision from [keyserver] Increase `social_proof` column of `cookies` table from `varchar(255)` to `varchar(1024)` to [keyserver] Increase `social_proof` column of `cookies` table from `varchar(255)` to `varchar(4096)`.Dec 31 2022, 4:10 PM

Harbormaster completed remote builds in B15013: Diff 20490.Dec 31 2022, 4:23 PM

Accepting to unblock but I think this is too large for a VARCHAR column and you should use TEXT. Please either address before landing, or link a Linear task to address it before landing

keyserver/src/database/migration-config.js
146 ↗	(On Diff #20490)	Is `VARCHAR(4096)` the right thing to use? Why not `TEXT`?

This revision is now accepted and ready to land.Jan 1 2023, 8:02 PM

In D6135#183964, @ashoat wrote:

Accepting to unblock but I think this is too large for a VARCHAR column and you should use TEXT. Please either address before landing, or link a Linear task to address it before landing

255 is the limit for VARCHAR to be one-byte length header + DATA. 65,532 is the limit for VARCHAR to be two-byte length header + DATA.

That said, there's no issue using TEXT instead. I'll update this diff.

I just saw that DEFAULT CHARSET=latin1;, wasn't sure what exactly that meant or the implications were, and went with VARCHAR because it seemed simpler?