Small(?) fixes to make PostgreSQL usable

[pe224]

Please confirm you have done the following before posting your bug report:

• I have enabled debug mode
• I have read checked the Common Issues page

Server (please complete the following information):

• Snipe-IT Version v4.7.8 build 4170 (g4fe689dc5)
• installed via Docker image
• Postgres 12.0

Describe the bug
Using the (inofficial) Postgres support, I have the feeling the functionality is 95% there.
The migrations are successful, which is awesome.
However, there are errors every time a boolean column needs to be read-out or updated, e.g. when I try to change the site name in admin/branding I get an Error: Please check the form below for errors.

The debug->session->errors shows

Illuminate\Support\ViewErrorBag {#814
  #bags: array:1 [
    "default" => Illuminate\Support\MessageBag {#806
      #messages: array:2 [
        "login_remote_user_enabled" => array:1 [
          0 => "The login remote user enabled must be a number."
        ]
        "login_common_disabled" => array:1 [
          0 => "The login common disabled must be a number."
        ]
      ]
      #format: ":message"
    }
  ]
}

the reason being the rules in app/Models/Setting.php

'login_remote_user_enabled' => 'numeric|nullable',
'login_common_disabled' => 'numeric|nullable',

The boolean values "true" and "false" in Postgres are not numeric, so the validation fails.

Now, I don't know PHP at all and simply changing the rules above to numeric|nullable|boolean also didn't work.
There seems to be some type incompatibility between boolean columns in MySQL and Postgres.
I have no idea how to fix this most easily and cleanly.

However, I noticed that some "boolean" flags, like settings.auto_increment_assets, are already represented as integer columns.
The following Postgres query converts all boolean columns in all tables to int2 columns, preserving values (including NULL) and defaults (and, for consistency reasons, converts the 5 existing "boolean" integer columns I discovered to int2):

DO
$$
DECLARE
  rec record;
BEGIN
  FOR rec IN
    SELECT table_name
          ,column_name
          ,CASE CAST(column_default AS BOOLEAN) WHEN TRUE THEN '1' WHEN FALSE THEN '0' ELSE 'NULL' END AS default_value
    FROM information_schema.columns
    WHERE data_type = 'boolean'
    AND table_name NOT LIKE 'pg_%'
  LOOP
    EXECUTE format('
        ALTER TABLE %I
           ALTER COLUMN %2$I DROP default
          ,ALTER COLUMN %2$I TYPE int2 USING CASE %2$I WHEN TRUE THEN 1 WHEN FALSE THEN 0 ELSE NULL END
          ,ALTER COLUMN %2$I SET DEFAULT %s;'
      ,rec.table_name, rec.column_name, rec.default_value);
  END LOOP;
END;
$$ LANGUAGE plpgsql;

ALTER TABLE settings ALTER COLUMN qr_code TYPE int2;
ALTER TABLE settings ALTER COLUMN display_asset_name TYPE int2;
ALTER TABLE settings ALTER COLUMN display_checkout_date TYPE int2;
ALTER TABLE settings ALTER COLUMN display_eol TYPE int2;
ALTER TABLE settings ALTER COLUMN auto_increment_assets TYPE int2;

It seems that after this, the most common functions are useable without problems.

I feel like this might be one last step missing before Postgres is usable, so although it's not officially supported, maybe it could be fixed.

I would also like to ask if it might be possible to include the package php7.1-pgsql in the official Dockerfile. Even though it will not be used or supported (yet), it would relieve people playing around with Postgres from building their own Docker images. I don't know what's the overhead in terms of image size for the additional driver, but I would assume it's bearable(?)

[hapm]

I would also like to ask if it might be possible to include the package php7.1-pgsql in the official Dockerfile. Even though it will not be used or supported (yet), it would relieve people playing around with Postgres from building their own Docker images. I don't know what's the overhead in terms of image size for the additional driver, but I would assume it's bearable(?)

I would suggest to provide an extra pgsql tagged image, so it doesn't affect other people using the default docker images.

[stale]

Is this still relevant? We haven't heard from anyone in a bit. If so, please comment with any updates or additional detail.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Don't take it personally, we just need to keep a handle on things. Thank you for your contributions!

[pe224]

I would still be interested in the proposed changes, but I understand that it's low on the priority list.
If there's no reaction by the next time stale bot complains, I'll just let it auto-close.

[stale]

Okay, it looks like this issue or feature request might still be important. We'll re-open it for now. Thank you for letting us know!

[stale]

Is this still relevant? We haven't heard from anyone in a bit. If so, please comment with any updates or additional detail.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Don't take it personally, we just need to keep a handle on things. Thank you for your contributions!

[stale]

This issue has been automatically closed because it has not had recent activity. If you believe this is still an issue, please confirm that this issue is still happening in the most recent version of Snipe-IT and reply to this thread to re-open it.

[re-rizaldywirawan]

i can confirm this issue is still relevant event in 5.1.8

this one is a fresh install, using pgsql and try to activate full multiple companies support.

[snipe]

@re-rizaldywirawan I'm happy to look at a PR for this, but as mentioned, PgSQL is not officially supported in Snipe-IT.

[uberbrady]

MySQL doesn't have a native boolean type. Just numbers.

I think it's certainly possible that it's as simple as -

If it's declared in the Migration as a Boolean, we need to validate as a boolean.

If it's declared in the Migration as a number, we need to validate it as a number.

Laravel's boolean validation is pretty loose - https://laravel.com/docs/8.x/validation#rule-boolean - so hopefully it should "just work" across both databases.

So I guess I'd try to find all the places in the code where we are (I suppose incorrectly) validating a boolean database column against a 'numeric' validation. That doesn't require any terrible database migrations or anything, which would be a relief. The change would be as simple as going from numeric|nullable to boolean|nullable.

There could absolutely be some really weird or wonky problems with the two slightly different interpretations of true/false - and those could be scary. But it's at least a start.

Validation - Laravel - The PHP Framework For Web Artisans

[snipe]

@uberbrady Totally get it, we just have higher priorities right now. :(

[pe224]

@uberbrady
Would you have any idea where to start? I could spare some time, as this would really help me to not spin up another DB server.

I also noticed a strange, inconsistent behavior. As described in my first post, when converting all boolean columns in Postgres to int2, everything actually works.
But something broke since then. I noticed that when changing the admin->branding settings, the checkboxes for Link to Snipe-IT in Emails and Allow user skin show inconsistent behavior.
Link to Snipe-IT in Emails when left unchecked writes false/0 to the DB
Allow user skin when leftunchecked writes NULL to the DB. This leads to a constraint error, as the allow_user_skin column in settings table is non-nullable.

I don't understand why it works for one checkbox, but not the other, as the form elements seem to be exactly identical.

[snipe]

It's something we'll consider, but it's a pretty low priority - and anything you'd do would need to be tested on MySQL and MariaDB, since those are the supported database types.