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

Added column size specification varchar(16000) to exporter SQL schema #33

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@huksley
Copy link
Contributor

huksley commented Nov 26, 2018

Self-explanatory. The current version does not allow to load even the simple BPMN files to WORKFLOW table.

@Zelldon

This comment has been minimized.

Copy link
Member

Zelldon commented Nov 27, 2018

Hi @huksley ,

thanks for your contribution.

The current version does not allow to load even the simple BPMN files to WORKFLOW table.

I think we tested it with H2 and it seems to work, but maybe this is true for other databases. What type of database do you use?

What is the reason for 16000 as value?
Maybe it makes sense to use CLOB instead of VARCHAR what do you think?

Greets
Chris

@Zelldon Zelldon self-requested a review Nov 27, 2018

@huksley

This comment has been minimized.

Copy link
Contributor

huksley commented Nov 27, 2018

H2 defaults to 255 symbols if not specified.
Usually BPMN processes are around 16k or less and split into different processes.
Unsure on using CLOB/TEXT because it can change application logic.

@Zelldon

This comment has been minimized.

Copy link
Member

Zelldon commented Nov 28, 2018

Hi @huksley ,

H2 defaults to 255 symbols if not specified.

I'm not sure if this is correct, since we have examples which are greater then 255 bytes anyhow
I think it makes sense to change this.

Usually BPMN processes are around 16k or less and split into different processes.

This is a bit vague, I mean why we say you can have only processes with maximal 16k bytes, not 17k or 18k? Would like to have an reason for this or no limit.

Unsure on using CLOB/TEXT because it can change application logic.

What do you mean with "it can change application logic"? You mean in the simple monitor?

Would like to have CLOB instead of VARCHAR, since this data type makes sense for larger values like the workflow models. Furthermore it is also ok if we change the current simple-monitor impl.

Would CLOB work for you? Which database do you use?

@huksley

This comment has been minimized.

Copy link
Contributor

huksley commented Nov 28, 2018

Hi I am using H2 and I have had an exception when uploading schema from simpe monitor that workflow.resource_ column can`t fit it due to column being 255 bytes.

After recreating table with resource_ varchar(16000) problem was fixed.

@huksley

This comment has been minimized.

Copy link
Contributor

huksley commented Nov 28, 2018

What do you mean with "it can change application logic"? You mean in the simple monitor?

CLOB datatype is not universally supported for example in MySQL/MariaDB it is named TEXT

@Zelldon

This comment has been minimized.

Copy link
Member

Zelldon commented Nov 29, 2018

Ok yes, but VARCHAR is not designed for this type of data (large texts). If the user wants to use another database, then he have to change it, maybe in the future we can also add an mechanism for auto detection.

The VARCHAR(16000) will also not work for oracle, since the max length is 4000 characters see here https://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT1822 or for ms sqlserver it is 8000, see here https://docs.microsoft.com/en-us/sql/t-sql/data-types/char-and-varchar-transact-sql?view=sql-server-2017 .

Merge pull request #1 from zeebe-io/master
Update from upstream
@huksley

This comment has been minimized.

Copy link
Contributor

huksley commented Dec 13, 2018

Superceeded by #39

@huksley huksley closed this Dec 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment