-
Notifications
You must be signed in to change notification settings - Fork 753
Add UI functionality to capture Oracle data source #1606
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
base: main
Are you sure you want to change the base?
Conversation
Added Oracle data source functionality in UI to leverage existing Wren Engine code
WalkthroughThe changes introduce Oracle as a supported data source throughout the codebase. This includes updates to enums, type definitions, and configuration mappings to recognize Oracle in both backend and frontend components. A new React component for Oracle connection properties is added, and utility functions are extended to handle Oracle-specific logic, images, and labels. The GraphQL schema and server-side data source handling are updated to process Oracle connection information. These updates collectively enable the application to support Oracle databases for configuration, connection, and display. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Backend
participant OracleDB
User->>UI: Selects Oracle as data source
UI->>UI: Renders OracleProperties form
User->>UI: Fills Oracle connection info
UI->>Backend: Submits Oracle connection info
Backend->>Backend: Decrypts and validates info
Backend->>OracleDB: Attempts connection using provided info
OracleDB-->>Backend: Responds with connection status
Backend-->>UI: Returns success or error
UI-->>User: Displays result
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
wren-ui/src/apollo/server/types/dataSource.ts (1)
23-27
: 🛠️ Refactor suggestionMissing Oracle properties interface in DataSourceProperties union type.
While ORACLE has been added to the DataSourceName enum, there is no corresponding OracleDataSourceProperties interface defined, and it's not included in the union type for DataSourceProperties. This might lead to type safety issues when handling Oracle connection properties.
export type DataSourceProperties = { displayName: string } & Partial< BigQueryDataSourceProperties & DuckDBDataSourceProperties & - PGDataSourceProperties + PGDataSourceProperties & + OracleDataSourceProperties >; +export interface OracleDataSourceProperties { + displayName: string; + host: string; + port: number; + database: string; + user: string; + password: string; + serviceName?: string; + ssl?: boolean; +}
🧹 Nitpick comments (3)
wren-ui/src/apollo/client/graphql/__types__.ts (1)
288-288
: ORACLE enum value placement breaks alphabetical ordering.The ORACLE value has been positioned between MSSQL and MYSQL, which breaks the alphabetical order present in the rest of the enum. Consider repositioning it to maintain consistency.
BIG_QUERY = 'BIG_QUERY', CLICK_HOUSE = 'CLICK_HOUSE', DUCKDB = 'DUCKDB', MSSQL = 'MSSQL', - ORACLE = 'ORACLE', MYSQL = 'MYSQL', + ORACLE = 'ORACLE', POSTGRES = 'POSTGRES', SNOWFLAKE = 'SNOWFLAKE', TRINO = 'TRINO'wren-ui/src/components/pages/setup/dataSources/OracleProperties.tsx (2)
41-53
: Add numeric validation for the port fieldThe port field should include validation to ensure it's a numeric value.
Add a validator to ensure the port is a number:
<Form.Item label="Port" name="port" required rules={[ { required: true, message: ERROR_TEXTS.CONNECTION.PORT.REQUIRED, }, + { + pattern: /^\d+$/, + message: 'Port must be a number', + } ]} > <Input placeholder="1521" disabled={isEditMode} /> </Form.Item>
54-65
: Add missing "required" attribute to Username Form.ItemThe Username field has validation rules requiring it, but is missing the "required" attribute on the Form.Item element.
Add the required attribute for consistent UI indication:
<Form.Item label="Username" name="user" + required rules={[ { required: true, message: ERROR_TEXTS.CONNECTION.USERNAME.REQUIRED, }, ]} > <Input /> </Form.Item>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
wren-ui/public/images/dataSource/oracle.svg
is excluded by!**/*.svg
📒 Files selected for processing (12)
wren-ui/src/apollo/client/graphql/__types__.ts
(1 hunks)wren-ui/src/apollo/server/adaptors/ibisAdaptor.ts
(2 hunks)wren-ui/src/apollo/server/dataSource.ts
(2 hunks)wren-ui/src/apollo/server/mdl/mdlBuilder.ts
(1 hunks)wren-ui/src/apollo/server/mdl/type.ts
(1 hunks)wren-ui/src/apollo/server/repositories/projectRepository.ts
(2 hunks)wren-ui/src/apollo/server/schema.ts
(1 hunks)wren-ui/src/apollo/server/types/dataSource.ts
(1 hunks)wren-ui/src/components/pages/setup/dataSources/OracleProperties.tsx
(1 hunks)wren-ui/src/components/pages/setup/utils.tsx
(1 hunks)wren-ui/src/utils/dataSourceType.ts
(4 hunks)wren-ui/src/utils/enum/dataSources.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
wren-ui/src/utils/dataSourceType.ts (1)
wren-ui/src/components/pages/setup/dataSources/OracleProperties.tsx (1)
OracleProperties
(10-97)
wren-ui/src/components/pages/setup/utils.tsx (1)
wren-ui/src/utils/dataSourceType.ts (1)
getDataSourceConfig
(87-93)
wren-ui/src/apollo/server/dataSource.ts (2)
wren-ui/src/apollo/server/repositories/projectRepository.ts (1)
ORACLE_CONNECTION_INFO
(35-41)wren-ui/src/apollo/server/adaptors/ibisAdaptor.ts (1)
HostBasedConnectionInfo
(30-36)
🔇 Additional comments (9)
wren-ui/src/utils/enum/dataSources.ts (1)
6-6
: Addition of ORACLE to DATA_SOURCES enum is consistent with project requirements.The addition of ORACLE to the DATA_SOURCES enum is well-positioned alphabetically between MYSQL and MSSQL, following the existing pattern in the enum.
wren-ui/src/apollo/server/types/dataSource.ts (1)
6-6
: Addition of ORACLE to DataSourceName enum is consistent.The addition of ORACLE to the DataSourceName enum follows the established pattern and maintains alphabetical ordering.
wren-ui/src/apollo/server/mdl/type.ts (1)
91-91
: Addition of ORACLE to WrenEngineDataSourceType enum is appropriate.The addition of ORACLE to the WrenEngineDataSourceType enum is consistent with the changes in other files and maintains the established pattern.
wren-ui/src/apollo/server/schema.ts (1)
52-52
: LGTM: Oracle added to DataSourceName enum.Oracle has been correctly added to the GraphQL schema's DataSourceName enum, aligning with the broader goal of supporting Oracle as a data source.
wren-ui/src/apollo/server/mdl/mdlBuilder.ts (1)
503-504
: LGTM: Oracle data source mapping added correctly.The Oracle data source type has been properly mapped to the corresponding WrenEngineDataSourceType.ORACLE in the buildDataSource method, following the same pattern as other data sources.
wren-ui/src/components/pages/setup/utils.tsx (1)
80-84
: LGTM: Oracle added to DATA_SOURCE_OPTIONS.Oracle has been properly added to the DATA_SOURCE_OPTIONS object with the correct configuration structure, including documentation link and enabled status.
wren-ui/src/apollo/server/adaptors/ibisAdaptor.ts (2)
82-82
: LGTM: Oracle added to SupportedDataSource enum.Oracle has been correctly added to the SupportedDataSource enum.
93-93
: LGTM: Oracle mapping added to dataSourceUrlMap.The mapping from SupportedDataSource.ORACLE to 'oracle' string value has been properly added to the dataSourceUrlMap.
wren-ui/src/utils/dataSourceType.ts (1)
5-5
:✅ Verification successful
LGTM: Oracle data source properly integrated
The Oracle data source is correctly integrated into all utility functions, following the established pattern for other data sources.
Also applies to: 20-21, 45-46, 70-71
🏁 Script executed:
#!/bin/bash # Check if Oracle SVG image exists if [ -f "wren-ui/public/images/dataSource/oracle.svg" ]; then echo "✅ Oracle SVG image exists" else echo "❌ Oracle SVG image is missing" # List available images to help determine where to place it find wren-ui/public/images/dataSource -type f -name "*.svg" fiLength of output: 117
Oracle SVG Asset Verified and Integration Approved
The
oracle.svg
icon has been confirmed inwren-ui/public/images/dataSource
, so the Oracle data source is fully integrated following the established pattern. LGTM.
// Oracle | ||
[DataSourceName.ORACLE]: { | ||
sensitiveProps: ['password'], | ||
toIbisConnectionInfo(connectionInfo) { | ||
const decryptedConnectionInfo = decryptConnectionInfo( | ||
DataSourceName.ORACLE, | ||
connectionInfo, | ||
); | ||
const { host, port, database, user, password } = | ||
decryptedConnectionInfo as ORACLE_CONNECTION_INFO; | ||
return { host, port, database, user, password }; | ||
}, | ||
} as IDataSourceConnectionInfo< | ||
ORACLE_CONNECTION_INFO, | ||
HostBasedConnectionInfo | ||
>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oracle implementation missing SSL handling
The implementation doesn't account for the SSL option that exists in the OracleProperties UI component. This will cause the SSL setting to be ignored when connecting to Oracle databases.
Reference the ClickHouse implementation (lines 180-199) on how to handle SSL parameters:
// Oracle
[DataSourceName.ORACLE]: {
sensitiveProps: ['password'],
toIbisConnectionInfo(connectionInfo) {
const decryptedConnectionInfo = decryptConnectionInfo(
DataSourceName.ORACLE,
connectionInfo,
);
- const { host, port, database, user, password } =
+ const { host, port, database, user, password, ssl } =
decryptedConnectionInfo as ORACLE_CONNECTION_INFO;
- return { host, port, database, user, password };
+ return {
+ host,
+ port,
+ database,
+ user,
+ password,
+ ...(ssl && { kwargs: { ssl: true } })
+ };
},
} as IDataSourceConnectionInfo<
ORACLE_CONNECTION_INFO,
HostBasedConnectionInfo
>,
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Oracle | |
[DataSourceName.ORACLE]: { | |
sensitiveProps: ['password'], | |
toIbisConnectionInfo(connectionInfo) { | |
const decryptedConnectionInfo = decryptConnectionInfo( | |
DataSourceName.ORACLE, | |
connectionInfo, | |
); | |
const { host, port, database, user, password } = | |
decryptedConnectionInfo as ORACLE_CONNECTION_INFO; | |
return { host, port, database, user, password }; | |
}, | |
} as IDataSourceConnectionInfo< | |
ORACLE_CONNECTION_INFO, | |
HostBasedConnectionInfo | |
>, | |
// Oracle | |
[DataSourceName.ORACLE]: { | |
sensitiveProps: ['password'], | |
toIbisConnectionInfo(connectionInfo) { | |
const decryptedConnectionInfo = decryptConnectionInfo( | |
DataSourceName.ORACLE, | |
connectionInfo, | |
); | |
const { host, port, database, user, password, ssl } = | |
decryptedConnectionInfo as ORACLE_CONNECTION_INFO; | |
return { | |
host, | |
port, | |
database, | |
user, | |
password, | |
...(ssl && { kwargs: { ssl: true } }), | |
}; | |
}, | |
} as IDataSourceConnectionInfo< | |
ORACLE_CONNECTION_INFO, | |
HostBasedConnectionInfo | |
>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the first comment alone. I'm not sure if ...(ssl && { kwargs: { ssl: true } }) would be necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@narsik123 I reviewed the code and can see that your OracleProperties component includes an SSL toggle switch:
<Form.Item
label={t('ssl')}
name="ssl"
valuePropName="checked"
tooltip={t('dataSource.sslTooltip')}
>
<Switch />
</Form.Item>
Since you've added this SSL option in the UI, you should handle it in the implementation to ensure the user's selection is respected when connecting to Oracle databases.
The suggested approach using ...(ssl && { kwargs: { ssl: true } })
follows the same pattern used by MS SQL Server in your codebase:
...(trustServerCertificate && { kwargs: { trustServerCertificate: 'YES' } })
This is the standard pattern in your codebase for passing optional connection parameters via the kwargs property when using host-based connections. Without this implementation, the SSL toggle in your UI won't have any effect on the actual connection.
I recommend including the SSL handling as suggested to maintain consistency with how other database types handle connection parameters.
🏁 Script executed:
#!/bin/bash
# Check if OracleProperties has SSL functionality
echo "=== OracleProperties SSL Check ==="
grep -A 5 -B 5 "ssl" wren-ui/src/components/pages/setup/dataSources/OracleProperties.tsx
# Check ORACLE_CONNECTION_INFO interface
echo -e "\n\n=== ORACLE_CONNECTION_INFO Interface Definition ==="
grep -A 10 "ORACLE_CONNECTION_INFO" wren-ui/src/apollo/server/repositories/projectRepository.ts
# Check how MS SQL (similar host-based connection) handles additional parameters
echo -e "\n\n=== MS SQL Implementation ==="
grep -A 15 -B 5 "\[DataSourceName.MSSQL\]" wren-ui/src/apollo/server/dataSource.ts
Length of output: 1917
🏁 Script executed:
#!/bin/bash
# Let's see the full OracleProperties component to understand the SSL implementation
cat wren-ui/src/components/pages/setup/dataSources/OracleProperties.tsx
Length of output: 2329
@narsik123 I've found an important inconsistency that needs addressing:
-
Your OracleProperties component has an SSL toggle:
<Form.Item label="Use SSL" name="ssl" valuePropName="checked"> <Switch /> </Form.Item>
-
But the ORACLE_CONNECTION_INFO interface doesn't include an ssl property:
export interface ORACLE_CONNECTION_INFO { host: string; port: number; user: string; password: string; database: string; }
This means even if users toggle SSL in the UI, that setting won't be captured in the connection info. You need to:
- First add the
ssl: boolean;
property to the ORACLE_CONNECTION_INFO interface - Then implement SSL handling in the connection logic
The suggested pattern ...(ssl && { kwargs: { ssl: true } })
follows how MS SQL handles its trustServerCertificate option and would be appropriate once the interface includes the ssl property.
Without these changes, the SSL toggle in your UI won't actually do anything.
🧬 Code Graph Analysis Results
Relevant snippets from wren-ui/src/apollo/server/repositories/projectRepository.ts
(lines 13-82)
These define the TypeScript interfaces for connection info objects used in the main file, including the ssl
property for Postgres and ClickHouse, and trustServerCertificate
for MS SQL:
export interface BIG_QUERY_CONNECTION_INFO {
projectId: string;
datasetId: string;
credentials: string;
}
export interface POSTGRES_CONNECTION_INFO {
host: string;
port: number;
user: string;
password: string;
database: string;
ssl: boolean;
}
export interface MYSQL_CONNECTION_INFO {
host: string;
port: number;
user: string;
password: string;
database: string;
}
export interface ORACLE_CONNECTION_INFO {
host: string;
port: number;
user: string;
password: string;
database: string;
}
export interface MS_SQL_CONNECTION_INFO {
host: string;
port: number;
user: string;
password: string;
database: string;
trustServerCertificate: boolean;
}
export interface CLICK_HOUSE_CONNECTION_INFO {
host: string;
port: number;
user: string;
password: string;
database: string;
ssl: boolean;
}
export interface TRINO_CONNECTION_INFO {
host: string;
port: number;
schemas: string;
username: string;
password: string;
ssl: boolean;
}
export interface SNOWFLAKE_CONNECTION_INFO {
user: string;
password: string;
account: string;
database: string;
schema: string;
}
export interface DUCKDB_CONNECTION_INFO {
initSql: string;
extensions: Array<string>;
configurations: Record<string, any>;
}
export type WREN_AI_CONNECTION_INFO =
| BIG_QUERY_CONNECTION_INFO
| POSTGRES_CONNECTION_INFO
| MYSQL_CONNECTION_INFO
| ORACLE_CONNECTION_INFO
| DUCKDB_CONNECTION_INFO
| MS_SQL_CONNECTION_INFO
| CLICK_HOUSE_CONNECTION_INFO
| TRINO_CONNECTION_INFO
| SNOWFLAKE_CONNECTION_INFO;
Relevant snippets from wren-ui/src/apollo/server/adaptors/ibisAdaptor.ts
(lines 30-67)
These define the Ibis connection info interfaces returned by the toIbisConnectionInfo
functions, showing the expected structure for Postgres, Trino, Snowflake, BigQuery, etc.:
export interface HostBasedConnectionInfo {
host: string;
port: number;
database: string;
user: string;
password: string;
}
export interface UrlBasedConnectionInfo {
connectionUrl: string;
}
export interface IbisBigQueryConnectionInfo {
project_id: string;
dataset_id: string;
credentials: string; // base64 encoded
}
export type IbisPostgresConnectionInfo =
| UrlBasedConnectionInfo
| HostBasedConnectionInfo;
export interface IbisTrinoConnectionInfo {
host: string;
port: number;
catalog: string;
schema: string;
user: string;
password: string;
}
export interface IbisSnowflakeConnectionInfo {
user: string;
password: string;
account: string;
database: string;
schema: string;
}
Summary of wren-ui/src/apollo/server/dataSource.ts
- The file defines a
dataSource
object keyed byDataSourceName
enum values. - Each data source entry specifies
sensitiveProps
(e.g.,password
,credentials
) and atoIbisConnectionInfo
method that converts the decrypted connection info into the expected Ibis connection info format. - For Postgres, the
toIbisConnectionInfo
method:- Decrypts the connection info.
- Extracts
host
,port
,database
,user
,password
, andssl
. - URL-encodes the password.
- Constructs a connection URL string including
sslmode=require
ifssl
is true.
- For MS SQL, a similar pattern is used, with an optional
kwargs
property added iftrustServerCertificate
is true. - For ClickHouse, the connection URL includes
secure=1
ifssl
is true. - For Trino, the host URL is prefixed with
https://
ifssl
is true, otherwisehttp://
. - The user comment references uncertainty about the necessity of
(ssl && { kwargs: { ssl: true } })
pattern, which is similar to the existing pattern used for MS SQL withtrustServerCertificate
.
Possibly relevant snippet from migration wren-ui/migrations/20240530062809_transfer_project_table_data.js
(lines 40-41)
const ssl =
project.configurations && project.configurations.ssl ? true : false;
This snippet shows how the ssl
flag might be set from project configurations, relevant to understanding how ssl
is used in connection info.
Possibly relevant snippet from wren-ui/src/apollo/server/config.ts
(lines 153-155)
export function getConfig(): IConfig {
return { ...defaultConfig, ...pickBy(config) };
}
This shows the getConfig
function used in dataSource.ts
to obtain configuration, which is then used to initialize the Encryptor
.
Possibly relevant snippet from wren-ui/src/pages/api/graphql.ts
(lines 24-28)
export const config: PageConfig = {
api: {
bodyParser: false,
},
};
This snippet is unrelated to connection info or SSL handling but is included for completeness.
[End of relevant snippets]
✏️ Learnings added
Learnt from: narsik123
PR: Canner/WrenAI#1606
File: wren-ui/src/apollo/server/dataSource.ts:135-150
Timestamp: 2025-04-28T20:09:06.069Z
Learning: The Oracle data source in WrenAI includes an SSL toggle in the UI (OracleProperties component) and should pass this setting to the connection info using the pattern `...(ssl && { kwargs: { ssl: true } })` for consistency with other host-based connections like MS SQL.
Learnt from: narsik123
PR: Canner/WrenAI#1606
File: wren-ui/src/apollo/server/dataSource.ts:135-150
Timestamp: 2025-04-28T20:09:06.069Z
Learning: The Oracle data source implementation has an inconsistency: the OracleProperties UI component includes an SSL toggle switch, but the ORACLE_CONNECTION_INFO interface doesn't include an ssl property. This means the SSL setting in the UI has no effect on the actual connection. The interface should be updated to include ssl:boolean, and the implementation should use the pattern `...(ssl && { kwargs: { ssl: true } })` for handling SSL.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
import { Form, Input, Switch } from 'antd'; | ||
import { ERROR_TEXTS } from '@/utils/error'; | ||
import { FORM_MODE } from '@/utils/enum'; | ||
import { hostValidator } from '@/utils/validator'; | ||
|
||
interface Props { | ||
mode?: FORM_MODE; | ||
} | ||
|
||
export default function OracleProperties(props: Props) { | ||
const { mode } = props; | ||
const isEditMode = mode === FORM_MODE.EDIT; | ||
return ( | ||
<> | ||
<Form.Item | ||
label="Display name" | ||
name="displayName" | ||
required | ||
rules={[ | ||
{ | ||
required: true, | ||
message: ERROR_TEXTS.CONNECTION.DISPLAY_NAME.REQUIRED, | ||
}, | ||
]} | ||
> | ||
<Input /> | ||
</Form.Item> | ||
<Form.Item | ||
label="Host" | ||
name="host" | ||
required | ||
rules={[ | ||
{ | ||
required: true, | ||
validator: hostValidator, | ||
}, | ||
]} | ||
> | ||
<Input placeholder="10.1.1.1" disabled={isEditMode} /> | ||
</Form.Item> | ||
<Form.Item | ||
label="Port" | ||
name="port" | ||
required | ||
rules={[ | ||
{ | ||
required: true, | ||
message: ERROR_TEXTS.CONNECTION.PORT.REQUIRED, | ||
}, | ||
]} | ||
> | ||
<Input placeholder="1521" disabled={isEditMode} /> | ||
</Form.Item> | ||
<Form.Item | ||
label="Username" | ||
name="user" | ||
rules={[ | ||
{ | ||
required: true, | ||
message: ERROR_TEXTS.CONNECTION.USERNAME.REQUIRED, | ||
}, | ||
]} | ||
> | ||
<Input /> | ||
</Form.Item> | ||
<Form.Item | ||
label="Password" | ||
name="password" | ||
required | ||
rules={[ | ||
{ | ||
required: true, | ||
message: ERROR_TEXTS.CONNECTION.PASSWORD.REQUIRED, | ||
}, | ||
]} | ||
> | ||
<Input.Password placeholder="input password" /> | ||
</Form.Item> | ||
<Form.Item | ||
label="Database name" | ||
name="database" | ||
required | ||
rules={[ | ||
{ | ||
required: true, | ||
message: ERROR_TEXTS.CONNECTION.DATABASE.REQUIRED, | ||
}, | ||
]} | ||
> | ||
<Input placeholder="Oracle database name" disabled={isEditMode} /> | ||
</Form.Item> | ||
<Form.Item label="Use SSL" name="ssl" valuePropName="checked"> | ||
<Switch /> | ||
</Form.Item> | ||
</> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Missing test file for the OracleProperties component
To ensure reliable functionality, a test file should be created to validate the component's behavior.
Create a test file for this component to verify:
- Form validation rules are applied correctly
- Fields are properly disabled in edit mode
- The component renders all required fields
Would you like me to generate a test file template for this component?
<Form.Item label="Use SSL" name="ssl" valuePropName="checked"> | ||
<Switch /> | ||
</Form.Item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SSL field is not defined in the ORACLE_CONNECTION_INFO interface
This form field collects SSL configuration, but the corresponding interface in projectRepository.ts doesn't have an SSL field to store this value.
Ensure the SSL toggle configuration is properly stored by adding the ssl field to the ORACLE_CONNECTION_INFO interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the SSL field to Oracle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
@narsik123 Thank you for addressing the issue! I've verified that you've added the SSL field to the ORACLE_CONNECTION_INFO interface, which ensures the SSL configuration from the UI form will be properly stored. This completes the implementation for Oracle data source support with proper SSL configuration handling.
🐰 happy hopping
🏁 Script executed:
#!/bin/bash
# Check the current state of the ORACLE_CONNECTION_INFO interface to verify the SSL field has been added
cat wren-ui/src/apollo/server/repositories/projectRepository.ts | grep -A 10 "interface ORACLE_CONNECTION_INFO"
Length of output: 320
@narsik123 I checked the ORACLE_CONNECTION_INFO interface in projectRepository.ts, but I don't see the SSL field there yet. The interface still only contains:
export interface ORACLE_CONNECTION_INFO {
host: string;
port: number;
user: string;
password: string;
database: string;
}
Your OracleProperties.tsx component includes an SSL toggle, but this value isn't being stored without the corresponding field in the interface. Could you add ssl?: boolean;
to the ORACLE_CONNECTION_INFO interface to complete the implementation?
🏁 Script executed:
#!/bin/bash
# Check if there are any other interfaces or types related to Oracle that might contain the SSL field
grep -r "interface.*Oracle" --include="*.ts" --include="*.tsx" .
Length of output: 66
@narsik123 I've been trying to locate where the SSL field might have been added for Oracle connections, but I can't find any evidence of this change in the ORACLE_CONNECTION_INFO interface.
The form component in OracleProperties.tsx includes an SSL toggle switch:
<Form.Item label="Use SSL" name="ssl" valuePropName="checked">
<Switch />
</Form.Item>
However, the corresponding interface still doesn't have an SSL field:
export interface ORACLE_CONNECTION_INFO {
host: string;
port: number;
user: string;
password: string;
database: string;
}
For the form's SSL toggle to work properly, you'll need to add ssl?: boolean;
to the ORACLE_CONNECTION_INFO interface. Could you confirm if this change is still pending, or if you've implemented it in a different way?
🏁 Script executed:
#!/bin/bash
# More comprehensive search for ORACLE_CONNECTION_INFO
grep -r "ORACLE_CONNECTION_INFO" --include="*.ts" --include="*.tsx" .
# Check if there are any recent changes or commits related to Oracle SSL
git log -p --grep="Oracle" --grep="SSL" -n 5
Length of output: 501
@narsik123 I've searched through the codebase but couldn't find evidence that the SSL field was added to the ORACLE_CONNECTION_INFO interface.
The OracleProperties component has a form field for SSL:
<Form.Item label="Use SSL" name="ssl" valuePropName="checked">
<Switch />
</Form.Item>
But the corresponding interface in projectRepository.ts doesn't have a matching field:
export interface ORACLE_CONNECTION_INFO {
host: string;
port: number;
user: string;
password: string;
database: string;
}
To properly store the SSL configuration, you'll need to add ssl?: boolean;
to this interface. Has this change been implemented in a different way, or is it still pending?
🧬 Code Graph Analysis Results
Relevant snippets from wren-ui/src/utils/validator/hostValidator.ts
(lines 3-13)
export const hostValidator = (_, value) => {
if (!value) {
return Promise.reject(ERROR_TEXTS.CONNECTION.HOST.REQUIRED);
}
if (['localhost', '127.0.0.1'].includes(value)) {
return Promise.reject(ERROR_TEXTS.CONNECTION.HOST.INVALID);
}
return Promise.resolve();
};
- This snippet defines a validator function for the "Host" field used in the OracleProperties form. It ensures the host is provided and is not a local address.
Relevant snippets from wren-ui/src/utils/error/dictionary.ts
(lines 1-141)
export const ERROR_TEXTS = {
CONNECTION: {
DISPLAY_NAME: {
REQUIRED: 'Please input display name.',
},
HOST: {
REQUIRED: 'Please input host.',
INVALID:
"Invalid host. Use 'host.docker.internal' on macOS/Windows to connect to the local database.",
},
PORT: {
REQUIRED: 'Please input port.',
},
USERNAME: {
REQUIRED: 'Please input username.',
},
PASSWORD: {
REQUIRED: 'Please input password.',
},
DATABASE: {
REQUIRED: 'Please input database name.',
},
// ... other error texts omitted for brevity
},
// ... other error categories omitted for brevity
};
- This snippet contains error message constants used for form validation in OracleProperties, including messages for required fields like display name, host, port, username, password, and database name.
[no other relevant code snippets]
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
Hey @narsik123 thanks for the contribution! This is really exciting. See if @fredalai or @andreashimin could take a look ? |
updated with CodeRabbit suggestions
@fredalai or @andreashimin could take a look ? Please let me know to integrate and test with your Oracle instance. |
@narsik123 ,@douenergy I merged this pull request and tried to run it, however it doesn't work, reason : https://github.com/Canner/wren-engine/blob/main/ibis-server/app/model/metadata/oracle.py "SYSTEM" user hardcoded in this file: WHERE |
thank you @numberonewastefellow. @chilijung @wwwy3y3 @goldmedal this would warrant a review of wren-engine code. Can I get some help in testing and if needed addressing the functionality in the engine. |
@narsik123 i can help you to test, by the way did you tested oracle UI with connection to oracle db? or your just pushing UI changes? I'm working actively fixing engine bugs. |
The Ui changes are functional. I'm able to fetch the tables and populate the model. But as you noted only the "system" tables are selected!! |
@narsik123 after populating tables, did you treid to run query and see search results? i don't see this: ERROR: It seems there is missing implementation in either IBIS Server or wren engine: @douenergy can you please check this error? |
That explains. I still rely on the docker image for ibis which likely relies on v2 connector.py and hence did not encounter this issue. Is there a reason that the main branch of WrenAI in GitHub still points to [wren-engine @ 922ec70] |
@narsik123 @numberonewastefellow I think we're missing some enum variants in wren-core for Thanks for reporting this! 🙌 |
@douenergy @narsik123 @numberonewastefellow
|
To, @goldmedal, @narsik123 @douenergy @goldmedal i merged your changes manually, not able to run, I also tried with your image: oracle-ds-1, in both cases same error. 2025-05-05 11:47:26.048 | [7084a25a-f643-4be0-a55c-d724e2e5cbee] | INFO | init.dispatch:29 - Request ended 2025-05-05 11:47:27.067 | [656d1131-f77f-492d-82f6-649d07cf79a8] | DEBUG | rewriter.rewrite:56 - Extracted manifest: eyJjYXRhbG9nIjoid3JlbmFpIiwic2NoZW1hIjoicHVibGljIiwibW9kZWxzIjpbeyJuYW1lIjoiU1lTVEVNX0hFTFAiLCJyZWZTcWwiOm51bGwsImJhc2VPYmplY3QiOm51bGwsInRhYmxlUmVmZXJlbmNlIjp7ImNhdGFsb2ciOm51bGwsInNjaGVtYSI6IlNZU1RFTSIsInRhYmxlIjoiSEVMUCJ9LCJjb2x1bW5zIjpbeyJuYW1lIjoiVE9QSUMiLCJ0eXBlIjoiVkFSQ0hBUiIsInJlbGF0aW9uc2hpcCI6bnVsbCwiaXNDYWxjdWxhdGVkIjpmYWxzZSwibm90TnVsbCI6dHJ1ZSwiZXhwcmVzc2lvbiI6IiIsImlzSGlkZGVuIjpmYWxzZSwicmxzIjpudWxsLCJjbHMiOm51bGx9LHsibmFtZSI6IlNFUSIsInR5cGUiOiJERUNJTUFMIiwicmVsYXRpb25zaGlwIjpudWxsLCJpc0NhbGN1bGF0ZWQiOmZhbHNlLCJub3ROdWxsIjp0cnVlLCJleHByZXNzaW9uIjoiIiwiaXNIaWRkZW4iOmZhbHNlLCJybHMiOm51bGwsImNscyI6bnVsbH0seyJuYW1lIjoiSU5GTyIsInR5cGUiOiJWQVJDSEFSIiwicmVsYXRpb25zaGlwIjpudWxsLCJpc0NhbGN1bGF0ZWQiOmZhbHNlLCJub3ROdWxsIjpmYWxzZSwiZXhwcmVzc2lvbiI6IiIsImlzSGlkZGVuIjpmYWxzZSwicmxzIjpudWxsLCJjbHMiOm51bGx9XSwicHJpbWFyeUtleSI6IiIsImNhY2hlZCI6ZmFsc2UsInJlZnJlc2hUaW1lIjpudWxsfV0sInJlbGF0aW9uc2hpcHMiOltdLCJtZXRyaWNzIjpbXSwidmlld3MiOltdLCJkYXRhU291cmNlIjoiT1JBQ0xFIn0= During handling of the above exception, another exception occurred: Traceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last): During handling of the above exception, another exception occurred: Traceback (most recent call last):
|
I tried to use the same request body to add a test case for reproducing in async def test_query_1(client, connection_info):
manifest_str = "eyJzY2hlbWEiOiJwdWJsaWMiLCJjYXRhbG9nIjoid3JlbmFpIiwiZGF0YVNvdXJjZSI6Ik9SQUNMRSIsIm1vZGVscyI6W3sibmFtZSI6IlNZU1RFTV9IRUxQIiwiY29sdW1ucyI6W3sibmFtZSI6IlRPUElDIiwidHlwZSI6IlZBUkNIQVIiLCJpc0NhbGN1bGF0ZWQiOmZhbHNlLCJub3ROdWxsIjp0cnVlLCJwcm9wZXJ0aWVzIjp7ImRpc3BsYXlOYW1lIjoiVE9QSUMifX0seyJuYW1lIjoiU0VRIiwidHlwZSI6IkRFQ0lNQUwiLCJpc0NhbGN1bGF0ZWQiOmZhbHNlLCJub3ROdWxsIjp0cnVlLCJwcm9wZXJ0aWVzIjp7ImRpc3BsYXlOYW1lIjoiU0VRIn19LHsibmFtZSI6IklORk8iLCJ0eXBlIjoiVkFSQ0hBUiIsImlzQ2FsY3VsYXRlZCI6ZmFsc2UsIm5vdE51bGwiOmZhbHNlLCJwcm9wZXJ0aWVzIjp7ImRpc3BsYXlOYW1lIjoiSU5GTyJ9fV0sInRhYmxlUmVmZXJlbmNlIjp7ImNhdGFsb2ciOm51bGwsInNjaGVtYSI6IlNZU1RFTSIsInRhYmxlIjoiSEVMUCJ9LCJjYWNoZWQiOmZhbHNlLCJwcm9wZXJ0aWVzIjp7ImRpc3BsYXlOYW1lIjoiU1lTVEVNLkhFTFAifSwicHJpbWFyeUtleSI6IiJ9XSwicmVsYXRpb25zaGlwcyI6W10sInZpZXdzIjpbXX0="
response = await client.post(
url=f"{base_url}/query",
json={
"connectionInfo": connection_info,
"manifestStr": manifest_str,
"sql": 'SELECT "TOPIC" FROM "SYSTEM_HELP" GROUP BY "TOPIC" ORDER BY COUNT(*) DESC',
},
headers={
X_WREN_FALLBACK_DISABLE: "true",
},
)
assert response.status_code == 200
result = response.json()
print(result) I noticed your SQL in the stdout log doesn't have the escaping character for the double-quotes.
I have no idea how to reproduce the same case. However, it should have the escaping character for the quotes like:
Another question, what's your oracle version? |
<Form.Item label="Use SSL" name="ssl" valuePropName="checked"> | ||
<Switch /> | ||
</Form.Item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove the Use SSL form field since Wren Engine doesn't support this now, please reference the https://docs.getwren.ai/oss/wren_engine_api#tag/OracleConnectionInfo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@narsik123 We could add it back after Wren Engine supports this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i see there are errors with data types: Data connection error
Decimal(precision=Decimal('2'), scale=Decimal('2'), nullable=True) has failed due to the following errors: precision: Decimal('2') is not either None or an int scale: Decimal('2') is not either None or an int Expected signature: Decimal(precision: Optional[int] = None, scale: Optional[int] = None, nullable: bool = True). It seems is failing for NUMBER 2,2
: salary NUMBER(8,2),
commission_pct NUMBER(2,2),
manager_id NUMBER(6),
department_id NUMBER(4),. Script to re-produce: REATE TABLE employees (
employee_id NUMBER(6),
first_name VARCHAR2(20),
last_name VARCHAR2(25) CONSTRAINT emp_last_name_nn NOT NULL,
email VARCHAR2(25) CONSTRAINT emp_email_nn NOT NULL,
phone_number VARCHAR2(20),
hire_date DATE CONSTRAINT emp_hire_date_nn NOT NULL,
job_id VARCHAR2(10) CONSTRAINT emp_job_nn NOT NULL,
salary NUMBER(8,2),
commission_pct NUMBER(2,2),
manager_id NUMBER(6),
department_id NUMBER(4),
CONSTRAINT emp_salary_min CHECK (salary > 0),
CONSTRAINT emp_email_uk UNIQUE (email)
);
Note: generated query able to execute outside wrenai. Please check logs for more details
Error Log:
------------------------------------------------------------------------------------------------------------
ibis-server-1 | 2025-05-06 09:15:50.368 | [eb89e34d-4a02-4dab-a30c-908771f80ff2] | INFO | connector.query:58 - EXECUTING THE SQL on DB: WITH "HR_EMPLOYEES" AS (SELECT "HR_EMPLOYEES"."EMPLOYEE_ID" AS "EMPLOYEE_ID", "HR_EMPLOYEES"."FIRST_NAME" AS "FIRST_NAME", "HR_EMPLOYEES"."LAST_NAME" AS "LAST_NAME", "HR_EMPLOYEES"."EMAIL" AS "EMAIL", "HR_EMPLOYEES"."PHONE_NUMBER" AS "PHONE_NUMBER", "HR_EMPLOYEES"."HIRE_DATE" AS "HIRE_DATE", "HR_EMPLOYEES"."JOB_ID" AS "JOB_ID", "HR_EMPLOYEES"."SALARY" AS "SALARY", "HR_EMPLOYEES"."COMMISSION_PCT" AS "COMMISSION_PCT", "HR_EMPLOYEES"."MANAGER_ID" AS "MANAGER_ID", "HR_EMPLOYEES"."DEPARTMENT_ID" AS "DEPARTMENT_ID" FROM (SELECT "HR_EMPLOYEES"."EMPLOYEE_ID" AS "EMPLOYEE_ID", "HR_EMPLOYEES"."FIRST_NAME" AS "FIRST_NAME", "HR_EMPLOYEES"."LAST_NAME" AS "LAST_NAME", "HR_EMPLOYEES"."EMAIL" AS "EMAIL", "HR_EMPLOYEES"."PHONE_NUMBER" AS "PHONE_NUMBER", "HR_EMPLOYEES"."HIRE_DATE" AS "HIRE_DATE", "HR_EMPLOYEES"."JOB_ID" AS "JOB_ID", "HR_EMPLOYEES"."SALARY" AS "SALARY", "HR_EMPLOYEES"."COMMISSION_PCT" AS "COMMISSION_PCT", "HR_EMPLOYEES"."MANAGER_ID" AS "MANAGER_ID", "HR_EMPLOYEES"."DEPARTMENT_ID" AS "DEPARTMENT_ID" FROM (SELECT "EMPLOYEE_ID" AS "EMPLOYEE_ID", "FIRST_NAME" AS "FIRST_NAME", "LAST_NAME" AS "LAST_NAME", "EMAIL" AS "EMAIL", "PHONE_NUMBER" AS "PHONE_NUMBER", "HIRE_DATE" AS "HIRE_DATE", "JOB_ID" AS "JOB_ID", "SALARY" AS "SALARY", "COMMISSION_PCT" AS "COMMISSION_PCT", "MANAGER_ID" AS "MANAGER_ID", "DEPARTMENT_ID" AS "DEPARTMENT_ID" FROM "HR"."EMPLOYEES" "HR_EMPLOYEES") "HR_EMPLOYEES") "HR_EMPLOYEES") SELECT "COMMISSION_PCT", "DEPARTMENT_ID", "EMAIL", "EMPLOYEE_ID", "FIRST_NAME", "HIRE_DATE", "JOB_ID", "LAST_NAME", "MANAGER_ID", "PHONE_NUMBER", "SALARY" FROM "HR_EMPLOYEES" FETCH FIRST 500 ROWS ONLY
wren-engine-1 | , "DEPARTMENT_ID" "DEPARTMENT_ID"
ibis-server-1 | 2025-05-06 09:15:50.369 | [eb89e34d-4a02-4dab-a30c-908771f80ff2] | INFO | connector.query:59 - Passing the limit: 500
wren-engine-1 | FROM
wren-engine-1 | "HR"."EMPLOYEES" "HR_EMPLOYEES"
wren-engine-1 | ) "HR_EMPLOYEES"
wren-engine-1 | ) "HR_EMPLOYEES"
wren-engine-1 | )
wren-engine-1 | SELECT
wren-engine-1 | "COMMISSION_PCT"
wren-engine-1 | , "DEPARTMENT_ID"
wren-engine-1 | , "EMAIL"
wren-engine-1 | , "EMPLOYEE_ID"
wren-engine-1 | , "FIRST_NAME"
wren-engine-1 | , "HIRE_DATE"
wren-engine-1 | , "JOB_ID"
wren-engine-1 | , "LAST_NAME"
wren-engine-1 | , "MANAGER_ID"
wren-engine-1 | , "PHONE_NUMBER"
wren-engine-1 | , "SALARY"
wren-engine-1 | FROM
wren-engine-1 | "HR_EMPLOYEES"
wren-engine-1 | LIMIT 500
wren-engine-1 |
ibis-server-1 | INFO 172.19.0.7:32852 - "POST /v3/connector/oracle/query?limit=500
ibis-server-1 | HTTP/1.1" 500
wren-ui-1 | [2025-05-06T09:15:50.543] [DEBUG] IbisAdaptor - Query error: Decimal(precision=Decimal('2'), scale=Decimal('2'), nullable=True) has failed due to the following errors:
wren-ui-1 | `precision`: Decimal('2') is not either None or an int
wren-ui-1 | `scale`: Decimal('2') is not either None or an int
wren-ui-1 |
wren-ui-1 | Expected signature: Decimal(precision: Optional[int] = None, scale: Optional[int] = None, nullable: bool = True)
ibis-server-1 | 2025-05-06 09:15:50.527 | [eb89e34d-4a02-4dab-a30c-908771f80ff2] | ERROR | __init__.dispatch:26 - Request failed
wren-ui-1 | sendEvent ibis_query_failed {
ibis-server-1 | Traceback (most recent call last):
wren-ui-1 | correlationId: undefined,
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/middleware/base.py", line 148, in call_next
wren-ui-1 | processTime: undefined,
ibis-server-1 | message = await recv_stream.receive()
wren-ui-1 | error: "Decimal(precision=Decimal('2'), scale=Decimal('2'), nullable=True) has failed due to the following errors:\n" +
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/anyio/streams/memory.py", line 126, in receive
wren-ui-1 | " `precision`: Decimal('2') is not either None or an int\n" +
ibis-server-1 | raise EndOfStream from None
wren-ui-1 | " `scale`: Decimal('2') is not either None or an int\n" +
ibis-server-1 | anyio.EndOfStream
wren-ui-1 | '\n' +
ibis-server-1 |
wren-ui-1 | 'Expected signature: Decimal(precision: Optional[int] = None, scale: Optional[int] = None, nullable: bool = True)',
ibis-server-1 | During handling of the above exception, another exception occurred:
wren-ui-1 | dataSource: 'ORACLE',
ibis-server-1 |
wren-ui-1 | sql: 'select "COMMISSION_PCT","DEPARTMENT_ID","EMAIL","EMPLOYEE_ID","FIRST_NAME","HIRE_DATE","JOB_ID","LAST_NAME","MANAGER_ID","PHONE_NUMBER","SALARY" from "HR_EMPLOYEES"'
ibis-server-1 | Traceback (most recent call last):
wren-ui-1 | } UNKNOWN false
ibis-server-1 | File "/app/.venv/bin/fastapi", line 8, in <module>
wren-ui-1 | [2025-05-06T09:15:50.544] [ERROR] APOLLO - == original error ==
ibis-server-1 | sys.exit(main())
wren-ui-1 | [2025-05-06T09:15:50.544] [ERROR] APOLLO - AxiosError: Request failed with status code 500
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/fastapi/cli.py", line 13, in main
wren-ui-1 | at settle (file:///app/node_modules/axios/lib/core/settle.js:19:12)
ibis-server-1 | cli_main()
wren-ui-1 | at IncomingMessage.handleStreamEnd (file:///app/node_modules/axios/lib/adapters/http.js:599:11)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/fastapi_cli/cli.py", line 348, in main
wren-ui-1 | at IncomingMessage.emit (node:events:529:35)
ibis-server-1 | app()
wren-ui-1 | at endReadableNT (node:internal/streams/readable:1400:12)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/typer/main.py", line 322, in __call__
wren-ui-1 | at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
ibis-server-1 | return get_command(self)(*args, **kwargs)
wren-ui-1 | at Axios.request (file:///app/node_modules/axios/lib/core/Axios.js:45:41)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/click/core.py", line 1161, in __call__
wren-ui-1 | at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
ibis-server-1 | return self.main(*args, **kwargs)
wren-ui-1 | at async f.query (/app/.next/server/chunks/980.js:1:1800)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/typer/core.py", line 740, in main
wren-ui-1 | at async c.ibisQuery (/app/.next/server/chunks/980.js:8:39835)
ibis-server-1 | return _main(
wren-ui-1 | at async c.preview (/app/.next/server/chunks/980.js:8:39012)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/typer/core.py", line 195, in _main
wren-ui-1 | at async N.previewModelData (/app/.next/server/pages/api/graphql.js:1:83066)
ibis-server-1 | rv = self.invoke(ctx)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/click/core.py", line 1697, in invoke
ibis-server-1 | return _process_result(sub_ctx.command.invoke(sub_ctx))
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/click/core.py", line 1443, in invoke
ibis-server-1 | return ctx.invoke(self.callback, **ctx.params)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/click/core.py", line 788, in invoke
ibis-server-1 | return __callback(*args, **kwargs)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/typer/main.py", line 697, in wrapper
ibis-server-1 | return callback(**use_params)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/fastapi_cli/cli.py", line 334, in run
ibis-server-1 | _run(
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/fastapi_cli/cli.py", line 162, in _run
ibis-server-1 | uvicorn.run(
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/uvicorn/main.py", line 579, in run
ibis-server-1 | server.run()
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/uvicorn/server.py", line 66, in run
ibis-server-1 | return asyncio.run(self.serve(sockets=sockets))
ibis-server-1 | File "/usr/local/lib/python3.11/asyncio/runners.py", line 190, in run
ibis-server-1 | return runner.run(main)
ibis-server-1 | File "/usr/local/lib/python3.11/asyncio/runners.py", line 118, in run
ibis-server-1 | return self._loop.run_until_complete(task)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/middleware/base.py", line 141, in coro
ibis-server-1 | await self.app(scope, receive_or_disconnect, send_no_error)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/middleware/base.py", line 179, in __call__
ibis-server-1 | response = await self.dispatch_func(request, call_next)
ibis-server-1 | > File "/app/middleware/__init__.py", line 24, in dispatch
ibis-server-1 | return await call_next(request)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/middleware/base.py", line 154, in call_next
ibis-server-1 | raise app_exc
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/middleware/base.py", line 141, in coro
ibis-server-1 | await self.app(scope, receive_or_disconnect, send_no_error)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/middleware/exceptions.py", line 62, in __call__
ibis-server-1 | await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
ibis-server-1 | raise exc
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
ibis-server-1 | await app(scope, receive, sender)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/routing.py", line 715, in __call__
ibis-server-1 | await self.middleware_stack(scope, receive, send)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/routing.py", line 735, in app
ibis-server-1 | await route.handle(scope, receive, send)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/routing.py", line 288, in handle
ibis-server-1 | await self.app(scope, receive, send)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/routing.py", line 76, in app
ibis-server-1 | await wrap_app_handling_exceptions(app, request)(scope, receive, send)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
ibis-server-1 | raise exc
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
ibis-server-1 | await app(scope, receive, sender)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/starlette/routing.py", line 73, in app
ibis-server-1 | response = await f(request)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/fastapi/routing.py", line 301, in app
ibis-server-1 | raw_response = await run_endpoint_function(
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/fastapi/routing.py", line 212, in run_endpoint_function
ibis-server-1 | return await dependant.call(**values)
ibis-server-1 | File "/app/routers/v3/connector.py", line 143, in query
ibis-server-1 | return await v2.connector.query(
ibis-server-1 | File "/app/routers/v2/connector.py", line 147, in query
ibis-server-1 | result = connector.query(rewritten_sql, limit=limit)
ibis-server-1 | File "/app/model/connector.py", line 60, in query
ibis-server-1 | return self._connector.query(sql, limit)
ibis-server-1 | File "/usr/local/lib/python3.11/contextlib.py", line 81, in inner
ibis-server-1 | return func(*args, **kwds)
ibis-server-1 | File "/app/model/connector.py", line 77, in query
ibis-server-1 | return self.connection.sql(sql).limit(limit).to_pandas()
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/ibis/backends/sql/__init__.py", line 179, in sql
ibis-server-1 | schema = self._get_schema_using_query(query)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/ibis/backends/oracle/__init__.py", line 594, in _get_schema_using_query
ibis-server-1 | typ = metadata_row_to_type(
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/ibis/backends/oracle/__init__.py", line 71, in metadata_row_to_type
ibis-server-1 | typ = dt.Decimal(precision=precision, scale=scale, nullable=nullable)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/ibis/common/bases.py", line 72, in __call__
ibis-server-1 | return cls.__create__(*args, **kwargs)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/ibis/common/grounds.py", line 119, in __create__
ibis-server-1 | kwargs = cls.__signature__.validate(cls, args, kwargs)
ibis-server-1 | File "/app/.venv/lib/python3.11/site-packages/ibis/common/annotations.py", line 501, in validate
ibis-server-1 | raise SignatureValidationError(
ibis-server-1 | ibis.common.annotations.SignatureValidationError: Decimal(precision=Decimal('2'), scale=Decimal('2'), nullable=True) has failed due to the following errors:
ibis-server-1 | `precision`: Decimal('2') is not either None or an int
ibis-server-1 | `scale`: Decimal('2') is not either None or an int
ibis-server-1 |
ibis-server-1 | Expected signature: Decimal(precision: Optional[int] = None, scale: Optional[int] = None, nullable: bool = True)
ibis-server-1 | 2025-05-06 09:15:50.529 | [eb89e34d-4a02-4dab-a30c-908771f80ff2] | INFO | __init__.dispatch:29 - Request ended
ibis-server-1 | ERROR Exception in ASGI application
ibis-server-1 | + Exception Group Traceback (most recent call last):
`
I also see errors with oracle functions like SYSDATE please find attached screenshots for reference.
let me know if you need any help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @numberonewastefellow,
As far as I know, Fix SQL should use Wren SQL, which is based on ANSI SQL, so... the SYSDATE function should not work.
@goldmedal Please feel free to correct me or provide additional information
Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fredalai @goldmedal just for information, the above sql is generated by wren, there are two issues in that. 1 is 'sysdate', another is "Decimal" if a column in oracle set as NUMBER(2,2) this Decimal error coming. above I attached full log. I'm also trying to understand how it is mapping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@numberonewastefellow For the Decimal
type issue, I found that it's an ibis issue. I will fix it in the ibis side by ibis-project/ibis#11200
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
regarding sysdate, wren is generated with "sysdate" in double quotes, i think we need to update function mapping in csv (ibis-server/resources/function-list/oracle)?
It's not related to the function list actually. The SQL is invalid in Oracle:
SQL> select * from t1 where c2 > add_months(sysdate, -360);
C1 C2
---------- ------------------
1 09-MAY-25
2 09-MAY-25
SQL> select * from t1 where c2 > add_months("sysdate", -360);
select * from t1 where c2 > add_months("sysdate", -360)
*
ERROR at line 1:
ORA-00904: "sysdate": invalid identifier
Help: https://docs.oracle.com/error-help/db/ora-00904/
So, I think we don't need to support the usage for sysdate
with quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@goldmedal @douenergy @narsik123 how this fix can be used in https://github.com/Canner/WrenAI/ ? It seems it is (https://github.com/ibis-project/ibis/pull/11200) separate project. can you please respond @fredalai. @fredalai which version is we are using now 1. https://github.com/Canner/wren-engine/tree/922ec70dd838b004fcb9e5e4be27a5da295a2c76/ibis-server/app
2. https://github.com/Canner/wren-engine/tree/main/ibis-server
i see we have two versions of ibis server, which is not matching with the https://github.com/ibis-project/ibis
i want to start develop from this place and start fixing bugs for oracle, but i can't really follow branches and version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@goldmedal regarding sysdate
I don't want to support sysdate
with quotes, however current wren-engine generates the query with "sysdate"
in double quotes, how to fix this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove the Use SSL form field since Wren Engine doesn't support this now, please reference the https://docs.getwren.ai/oss/wren_engine_api#tag/OracleConnectionInfo.
@fredalai I have removed the ssl form field and committed that change
Added Oracle data source functionality in UI to leverage existing Wren Engine code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation