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

Fix#1947 #1960

Merged
merged 4 commits into from
Mar 19, 2024
Merged

Fix#1947 #1960

merged 4 commits into from
Mar 19, 2024

Conversation

emrahtoy
Copy link
Contributor

This PR fixes 2 problem

  1. type problem while calculating "a month ago", old code were return timestamp in numbers but it must be Date
  2. Migrations to the uuid and timestamp with time zone Entity types with migration file

This PR adds 2 files

  1. util file for getting data source to generate typeorm migrations
  2. Migration file for type changes

This PR adds a npm command

  1. in Server package "typeorm:migration-generate" added to have easier typeorm data source with the help of new utility file

@emrahtoy
Copy link
Contributor Author

NOTE : This PR does not include Sqllite migration.

@HenryHengZJ HenryHengZJ linked an issue Mar 18, 2024 that may be closed by this pull request
@HenryHengZJ
Copy link
Contributor

coicidentally this PR also fixing the same issue

@HenryHengZJ
Copy link
Contributor

@emrahtoy here's a list of supported types - https://orkhan.gitbook.io/typeorm/docs/entities#column-types

type uuid supports

  • postgres
  • mysql
  • sqlite

type timestamp with time zone supports

  • postgres
  • mysql
  • sqlite

I tested sqlite it seems to work, maybe because of the Init.ts migration script so it ignores the column types specified in entities. Have you tried on mysql?

@emrahtoy
Copy link
Contributor Author

emrahtoy commented Mar 18, 2024

@emrahtoy here's a list of supported types - https://orkhan.gitbook.io/typeorm/docs/entities#column-types

type uuid supports

  • postgres
  • mysql
  • sqlite

type timestamp with time zone supports

  • postgres
  • mysql
  • sqlite

I tested sqlite it seems to work, maybe because of the Init.ts migration script so it ignores the column types specified in entities. Have you tried on mysql?

uuid becomes varchar find default that is why sqlite doesn't care about it.
timestamp with timezone type is not necessary as it is handled on the code side by returning actual date instead of numbers.

I haven't tested it on MySQL as it does not exist in my stack.

It seems like both PR are almost identical but mine has changed all the entities.

@HenryHengZJ
Copy link
Contributor

@emrahtoy here's a list of supported types - https://orkhan.gitbook.io/typeorm/docs/entities#column-types
type uuid supports

  • postgres
  • mysql
  • sqlite

type timestamp with time zone supports

  • postgres
  • mysql
  • sqlite

I tested sqlite it seems to work, maybe because of the Init.ts migration script so it ignores the column types specified in entities. Have you tried on mysql?

uuid becomes varchar find default that is why sqlite doesn't care about it. timestamp with timezone type is not necessary as it is handled on the code side by returning actual date instead of numbers.

I haven't tested it on MySQL as it does not exist in my stack.

It seems like both PR are almost identical but mine has changed all the entities.

okay, in that case can we get rid of the timestamp with time zone as its not necessary and I'm afraid we will bump into issues supporting mysql.

@emrahtoy
Copy link
Contributor Author

@emrahtoy here's a list of supported types - https://orkhan.gitbook.io/typeorm/docs/entities#column-types
type uuid supports

  • postgres
  • mysql
  • sqlite

type timestamp with time zone supports

  • postgres
  • mysql
  • sqlite

I tested sqlite it seems to work, maybe because of the Init.ts migration script so it ignores the column types specified in entities. Have you tried on mysql?

uuid becomes varchar find default that is why sqlite doesn't care about it. timestamp with timezone type is not necessary as it is handled on the code side by returning actual date instead of numbers.
I haven't tested it on MySQL as it does not exist in my stack.
It seems like both PR are almost identical but mine has changed all the entities.

okay, in that case can we get rid of the timestamp with time zone as its not necessary and I'm afraid we will bump into issues supporting mysql.

Yes for sure we can use timestamp

@emrahtoy
Copy link
Contributor Author

I have just changed timestamp with timezone sections to timestamp

@HenryHengZJ
Copy link
Contributor

HenryHengZJ commented Mar 19, 2024

@emrahtoy i dont think timestamp is supported for sqlite. Actually do we really need the column type there?

@emrahtoy
Copy link
Contributor Author

@emrahtoy i dont think timestamp is supported for sqlite. Actually do we really need the column type there?

I believe giving a type is important. It gives the opportunity of being future proof.
Decorators has been handling different depending on the driver you pick ( MySQL, sqlite etc.. ) with this way TypeOrm know what to do with the value and does the necessary conversions.

Even if it doesn't give support at all we are still able to create our own custom handlers.

@HenryHengZJ
Copy link
Contributor

thanks @emrahtoy !

@HenryHengZJ HenryHengZJ merged commit ec1bbc8 into FlowiseAI:main Mar 19, 2024
2 checks passed
gagaboy added a commit to gagaboy/Flowise that referenced this pull request Mar 20, 2024
* commit '57efa25fe59c0e4041281a0f8561b110ea691fe0': (521 commits)
  🥳 flowise@1.6.2 release
  🥳 flowise-ui@1.6.2 release
  🥳 flowise-components@1.6.4 release
  Feature/OpenAI Tool Agent (FlowiseAI#1991)
  Fix#1947 (FlowiseAI#1960)
  Update autoSyncSingleCommit.yml
  Update autoSyncSingleCommit.yml (FlowiseAI#1985)
  Feature/delete backspace (FlowiseAI#1983)
  Send valid JSON to autosync workflows for multi-line commit messages (FlowiseAI#1981)
  Feature/Custom Tool OverrideConfig (FlowiseAI#1979)
  Feature/Mistral FunctionAgent (FlowiseAI#1912)
  update the version number for AzureChatOpenAI
  Adding support for AzureChatOpenAI
  Update pnpm-lock.yaml
  Update lockFileVersion
  Enable github workflow pnpm caching
  update lock file
  update lock file
  Merge branch 'main' into feature/Indexing
  Set embed versions to latest (FlowiseAI#1957)
  ...
@neliocaa
Copy link

when changing the table to uuid, whoever is upgrading receives an error, since they already have data in another format, especially when it comes externally,
So, changing the chatId and sessionId fields to uuid doesn't make sense, and causes a bug in the upgrade

@emrahtoy
Copy link
Contributor Author

In my case, they were always UUID. @HenryHengZJ which one of these chatId and sessionId is actually a user variable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Operator does not exist error when opening chat window
3 participants