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

Procedures input and output parameters do not support TYPE OF COLUMN #169

Closed
luronumen opened this issue Feb 16, 2021 · 32 comments
Closed

Comments

@luronumen
Copy link

luronumen commented Feb 16, 2021

ACTUAL RESULT

  • Procedures input and output parameters do not support TYPE OF COLUMN:

ALTER PROCEDURE USER_SEARCH (
ID BIGINT )
RETURNS (
USER_NAME VARCHAR(32),
FIRST_NAME VARCHAR(32),
LAST_NAME VARCHAR(32),
SITE_ID BIGINT,
DBA TYPE OF VISIBLE,
KEEPALIVE_TIME TIMESTAMP,
VISIBLE TYPE OF VISIBLE,
FULL_NAME VARCHAR(100) )

EXPECTED RESULT

  • Procedures input and output parameters should support TYPE OF COLUMN:

ALTER PROCEDURE USER_SEARCH (
ID TYPE OF COLUMN USERS.ID )
RETURNS (
USER_NAME TYPE OF COLUMN USERS.USER_NAME,
FIRST_NAME TYPE OF COLUMN USERS.FIRST_NAME,
LAST_NAME TYPE OF COLUMN USERS.LAST_NAME,
SITE_ID TYPE OF COLUMN USERS.SITE_ID,
DBA TYPE OF COLUMN USERS.DBA,
KEEPALIVE_TIME TYPE OF COLUMN USERS.KEEPALIVE_TIME,
VISIBLE OF COLUMN USERS.VISIBLE,
FULL_NAME TYPE OF COLUMN USERS.FULL_NAME )

STEPS TO REPRODUCE THIS ISSUE
1- Create a Firebird 3.0 database with the following DDL instruction: FB30_DDL.txt

  • ISSUE 1:
    Right mouse click on USER_SEARCH procedure and then select Alter; input and output parameters are not showed as TYPE OF COLUMN
  • ISSUE 2:
    Double mouse click on USER_SEARCH procedure to Open the procedure property; input and output parameters are not showed as TYPE OF COLUMN.

IMPORTANT NOTES
For more details about the Use of Column Type in Declarations please check:
https://firebirdsql.org/file/documentation/html/en/refdocs/fblangref25/firebird-25-language-reference.html#fblangref25-ddl-proc-paramscoltype

Jdochoa added a commit to Jdochoa/flamerobin that referenced this issue Mar 1, 2021
…TYPE OF COLUMN mariuz#169

- Wrong "Create Selectable SP"  and "Create Selectable EB" output mariuz#78
@luronumen
Copy link
Author

Retest result on FlameRobin v0.9.3.7: FAILED!

ACTUAL RESULT

  • Not all input and output parameters are supporting TYPE OF COLUMN:

ALTER PROCEDURE USER_SEARCH (
ID BIGINT )
RETURNS (
USER_NAME VARCHAR(32),
FIRST_NAME VARCHAR(32),
LAST_NAME VARCHAR(32),
SITE_ID BIGINT,
DBA TYPE OF COLUMN USERS.DBA,
KEEPALIVE_TIME TIMESTAMP,
VISIBLE TYPE OF COLUMN USERS.VISIBLE,
FULL_NAME VARCHAR(100) )

@arvanus
Copy link
Collaborator

arvanus commented May 20, 2021

@Jdochoa
Expected:

create or alter function FN_NAME (
    FIELD1 MYDOMAN)
returns OTHERDOMAIN
as
declare variable variable ANOTHERDOMAIN;
begin
end

Actual:

create or alter function FN_NAME (
    FIELD1 TYPE OF COLUMN NUMERIC(14,0) )
RETURNS   OTHERDOMAIN
AS
declare variable type of ANOTHERDOMAIN;
BEGIN
END

@Jdochoa
Copy link
Contributor

Jdochoa commented May 22, 2021

@Jdochoa
Expected:

create or alter function FN_NAME (
    FIELD1 MYDOMAN)
returns OTHERDOMAIN
as
declare variable variable ANOTHERDOMAIN;
begin
end

Actual:

create or alter function FN_NAME (
    FIELD1 TYPE OF COLUMN NUMERIC(14,0) )
RETURNS   OTHERDOMAIN
AS
declare variable type of ANOTHERDOMAIN;
BEGIN
END

Hi @arvanus.

i can't reproduce it, do you have more information?

./jo

@luronumen
Copy link
Author

@Jdochoa
Expected:

create or alter function FN_NAME (
    FIELD1 MYDOMAN)
returns OTHERDOMAIN
as
declare variable variable ANOTHERDOMAIN;
begin
end

Actual:

create or alter function FN_NAME (
    FIELD1 TYPE OF COLUMN NUMERIC(14,0) )
RETURNS   OTHERDOMAIN
AS
declare variable type of ANOTHERDOMAIN;
BEGIN
END

Hi @arvanus

Using this flamerobin.7z I also NOT able to reproduce the issue that you mentioned. Which Firebird version are you using?

@arvanus
Copy link
Collaborator

arvanus commented May 24, 2021

Sorry, forgot to mention "type of", try with these:


SET TERM ^ ;

create or alter function DUMMY_FN (
    NEW_PARAM type of MYDOMAN)
returns integer
as
begin
  /* Function Text */
  return 1;
end^

SET TERM ; ^


@luronumen
Copy link
Author

Sorry, forgot to mention "type of", try with these:


SET TERM ^ ;

create or alter function DUMMY_FN (
    NEW_PARAM type of MYDOMAN)
returns integer
as
begin
  /* Function Text */
  return 1;
end^

SET TERM ; ^

Hi @arvanus

I can now reproduce the issue that you mentioned.

Best regards,
Luciano

@Jdochoa
Copy link
Contributor

Jdochoa commented May 25, 2021

Sorry, forgot to mention "type of", try with these:


SET TERM ^ ;

create or alter function DUMMY_FN (
    NEW_PARAM type of MYDOMAN)
returns integer
as
begin
  /* Function Text */
  return 1;
end^

SET TERM ; ^

Fix it.

@luronumen
Copy link
Author

Hi @Jdochoa

Could you generate a new build of FlameRobin so that I can help in validating the open issue?

Thanks in advance,
Luciano

@Jdochoa
Copy link
Contributor

Jdochoa commented May 25, 2021

Luciano

flamerobin.zip

./jo

@arvanus
Copy link
Collaborator

arvanus commented May 25, 2021

Looks Fixed to me

@luronumen
Copy link
Author

Luciano

flamerobin.zip

./jo

Hi @Jdochoa

This FlameRobin version do not list the procedures Input/Output parameters:

FlameRobin

@Jdochoa
Copy link
Contributor

Jdochoa commented May 28, 2021

Luciano

flamerobin.zip
./jo

Hi @Jdochoa

This FlameRobin version do not list the procedures Input/Output parameters:

FlameRobin

Hi @luronumen.

I can't reproduce it, please verify your preferences
imagen

Or wait the next snapshot.

@luronumen
Copy link
Author

Hi @Jdochoa

After set Show columns and parameters in tree this issue is NOT reproducible!
Thank you very much!

Best regards,
Luciano

@luronumen
Copy link
Author

Hi @Jdochoa

Retest result on flamerobin.zip:

  • ISSUE 1: FIXED!
  • ISSUE 2: It is still reproducible:

FlameRobin_Issue2

@Jdochoa
Copy link
Contributor

Jdochoa commented Jun 7, 2021

Hi @Jdochoa

Retest result on flamerobin.zip:

* ISSUE 1: FIXED!

* ISSUE 2: It is still reproducible:

FlameRobin_Issue2

Hi @luronumen.

I'm not sure about this, Flamerobin behaves like this:
In the preferences
imagen

If Only Show Datatype is checked, show this.
imagen

If Only Show domains is checked, show this.
imagen

If Show Boot domain and Datatype is checked, show this
imagen

In which scenario should I check type of?

./jo

@luronumen
Copy link
Author

luronumen commented Jun 7, 2021

Hi @Jdochoa

For the fields of a procedure that are defined as a TYPE OF COLUMN instead of the traditional field types (varchar, int, float, etc) I think it makes more sense to always show < tablename > . < columnname > instead of the traditional types (varchar, int, float, etc.)

Based on this I believe that we should check if a field of a procedure is TYPE OF COLUMN in all three possibilities:

  • Only show datatype
  • Only show domain
  • Show Both domain and datatype

Does it make sense to you too?

Jdochoa added a commit to Jdochoa/flamerobin that referenced this issue Jun 12, 2021
- Procedures input and output parameters do not support TYPE OF COLUMN mariuz#169
@Jdochoa
Copy link
Contributor

Jdochoa commented Jun 12, 2021

Hi @Jdochoa

For the fields of a procedure that are defined as a TYPE OF COLUMN instead of the traditional field types (varchar, int, float, etc) I think it makes more sense to always show < tablename > . < columnname > instead of the traditional types (varchar, int, float, etc.)

Based on this I believe that we should check if a field of a procedure is TYPE OF COLUMN in all three possibilities:

* Only show datatype

* Only show domain

* Show Both domain and datatype

Does it make sense to you too?

Hi @luronumen

It is modified so that instead of DataType, typeOf is displayed, please review at https://github.com/Jdochoa/flamerobin.git

./jo

@luronumen
Copy link
Author

Hi @Jdochoa

Could you generate a new build of FlameRobin so that I can validating the fix?

Thanks in advance,
Luciano

@luronumen
Copy link
Author

Hi @Jdochoa

I got to retest using the build that you mentioned on #210

Is it possible to show only TABLE.COLUMN instead of TYPE OF COLUMN TABLE.COLUMN?

FlameRobin

@luronumen
Copy link
Author

Hi @Jdochoa

The build that you mentioned on #210 is showing the following issue when I try to select the Functions Dependencies tab:

FlameRobin_FunctionsError

@Jdochoa
Copy link
Contributor

Jdochoa commented Jun 14, 2021

Hi @Jdochoa

The build that you mentioned on #210 is showing the following issue when I try to select the Functions Dependencies tab:

FlameRobin_FunctionsError

Thank you @luronumen, Fix it.

./jo

@Jdochoa
Copy link
Contributor

Jdochoa commented Jun 14, 2021

Hi @Jdochoa

I got to retest using the build that you mentioned on #210

Is it possible to show only TABLE.COLUMN instead of TYPE OF COLUMN TABLE.COLUMN?

FlameRobin

Hi @luronumen

It's possible, but not easy. we need some changes in the datatypes.
You decide if waiting or rollback the change.

./jo

@luronumen
Copy link
Author

Hi @Jdochoa

No problem for me to wait.
As soon as you have a version that fixes all these issue, please share the build here so I can help you to retest it.
Thank you very much for the great job you've done to modernize FlameRobin.
Lets fix all the issues, complete support for Firebird 3.0, and then move on to Firebird 4.0.

Best Regards,
Luciano

@arvanus
Copy link
Collaborator

arvanus commented Jun 14, 2021

Hello everyone
I would create a new column "Type of", that receives a Boolean value (checkbox), like IBexpert does. What do you think?

@luronumen
Copy link
Author

Hello everyone
I would create a new column "Type of", that receives a Boolean value (checkbox), like IBexpert does. What do you think?

Hi @arvanus

Did you mean replace the true/false strings in the grid with a checkbox in the Boolean type representation?
That would be wonderful!

Best Regards,
Luciano

@arvanus
Copy link
Collaborator

arvanus commented Jun 14, 2021

I mean like this:
image

@luronumen
Copy link
Author

Now I understand @arvanus !
In this case I think it would be better to just put a column called Type of Column/Domain between the Type and Not Null columns. What do you think?

FlameRobinType

@Jdochoa
Copy link
Contributor

Jdochoa commented Jun 14, 2021

Now I understand @arvanus !
In this case I think it would be better to just put a column called Type of Column/Domain between the Type and Not Null columns. What do you think?

FlameRobinType

Hi @arvanus
do you explain the functionality?

./jo

@arvanus
Copy link
Collaborator

arvanus commented Jun 14, 2021

Sorry @Jdochoa, I didn't understand what you meant
Today Flamerobin show the data in a different way comparing to IBExpert, FR simply shows the type(DOMAIN) together, while IB splits the data. I'd do the same as IB, splitting Type into 3 columns: Type, Type of, Domain/Column
Another alternative, is to keep "Type" as is (without the text TYPE OF), and simply add a column "Type of" as a boolean between "parameter" and "Type"

@luronumen
Copy link
Author

Hi @arvanus and @Jdochoa

What do you think about this two columns:

  • Type: Always show the native type of the Procedure/Function parameter (BIGINT, VARCHAR, BOOLEAN, etc);
  • Type of Column/Domain: Show domain or table column if Procedure/Function parameter is of this type;

@luronumen
Copy link
Author

Hi @Jdochoa

The following issue is happening when I try to delete a record:

FlameRobin_Delete

Jdochoa added a commit to Jdochoa/flamerobin that referenced this issue Mar 4, 2022
@luronumen
Copy link
Author

Retest result on FlameRobin 0.9.3.12: PASS!
Thank you very much for the fix!

Best Regards,
Luciano

au2010 pushed a commit to au2010/flamerobin that referenced this issue Aug 8, 2022
* master: (30 commits)
  Update codeql-analysis.yml
  Update _ibpp.cpp
  Update fr_settings.confdef
  Fix NO PAD collation bug
  Fix minor error.
  Update ExecuteSqlFrame.cpp
  Fix: mariuz#260
  Fix mariuz#68
  minor fix
  Show error msg when save SQL file failed
  configuration for FB library is added (Application level)
  Library per conexión
  Add 32768 for restore with Firebird 4 mariuz#242
  Import patchs from IBPP in SourceForge mariuz#215
  Alter Database Linger value mariuz#217
  Increment year to 2022
  Fix: Cosmetic issue with full DDL extraction mariuz#243
  Fix:  Incorrect trigger state display mariuz#252
  Fix:  Primary Keys and Foreign Keys are not listed on Indexes node mariuz#180
  Fix: Procedures input and output parameters do not support TYPE OF COLUMN mariuz#169
  ...
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

No branches or pull requests

3 participants