ocket8888 commented on code in PR #6544:
URL: https://github.com/apache/trafficcontrol/pull/6544#discussion_r843199961
##########
traffic_ops/traffic_ops_golang/dbhelpers/db_helpers.go:
##########
@@ -1788,3 +1788,102 @@ func GetRegionNameFromID(tx *sql.Tx, regionID int)
(string, bool, error) {
}
return regionName, true, nil
}
+
+// GetCommonServerPropertiesFromV4 converts CommonServerPropertiesV40 to
CommonServerProperties struct
+func GetCommonServerPropertiesFromV4(s tc.ServerV40, tx *sql.Tx)
(tc.CommonServerProperties, error) {
+ var id int
+ var desc string
+ rows, err := tx.Query("SELECT id, description from profile WHERE
name=$1", (s.ProfileNames)[0])
+ if err != nil {
+ return tc.CommonServerProperties{}, fmt.Errorf("querying
profile id and description by profile_name: " + err.Error())
+ }
+ defer log.Close(rows, "closing rows in GetCommonServerPropertiesFromV4")
+
+ for rows.Next() {
+ if err := rows.Scan(&id, &desc); err != nil {
+ return tc.CommonServerProperties{},
fmt.Errorf("scanning profile: " + err.Error())
+ }
+ }
+
+ return tc.CommonServerProperties{
+ Cachegroup: s.Cachegroup,
+ CachegroupID: s.CachegroupID,
+ CDNID: s.CDNID,
+ CDNName: s.CDNName,
+ DeliveryServices: s.DeliveryServices,
+ DomainName: s.DomainName,
+ FQDN: s.FQDN,
+ FqdnTime: s.FqdnTime,
+ GUID: s.GUID,
+ HostName: s.HostName,
+ HTTPSPort: s.HTTPSPort,
+ ID: s.ID,
+ ILOIPAddress: s.ILOIPAddress,
+ ILOIPGateway: s.ILOIPGateway,
+ ILOIPNetmask: s.ILOIPNetmask,
+ ILOPassword: s.ILOPassword,
+ ILOUsername: s.ILOUsername,
+ LastUpdated: s.LastUpdated,
+ MgmtIPAddress: s.MgmtIPAddress,
+ MgmtIPGateway: s.MgmtIPGateway,
+ MgmtIPNetmask: s.MgmtIPNetmask,
+ OfflineReason: s.OfflineReason,
+ Profile: &(s.ProfileNames)[0],
+ ProfileDesc: &desc,
+ ProfileID: &id,
+ PhysLocation: s.PhysLocation,
+ PhysLocationID: s.PhysLocationID,
+ Rack: s.Rack,
+ RevalPending: s.RevalPending,
+ Status: s.Status,
+ StatusID: s.StatusID,
+ TCPPort: s.TCPPort,
+ Type: s.Type,
+ TypeID: s.TypeID,
+ UpdPending: s.UpdPending,
+ XMPPID: s.XMPPID,
+ XMPPPasswd: s.XMPPPasswd,
+ }, nil
+}
+
+// UpdateServerProfilesForV4 updates server_profile table via update function
for APIv4
+func UpdateServerProfilesForV4(id *int, profile []string, tx *sql.Tx) error {
+ var profileNames []string
+
+ //Delete existing rows from server_profile to get the priority correct
for profile_name changes
+ _, err := tx.Exec("DELETE FROM server_profile WHERE server=$1", *id)
+ if err != nil {
+ return fmt.Errorf("updating server_profile by server id: %v" +
strconv.Itoa(*id) + ", error: " + err.Error())
+ }
+
+ for i, pName := range profile {
+ query := `INSERT INTO server_profile (server, profile_name,
priority) VALUES ($1, $2, $3)`
+ _, err := tx.Exec(query, *id, pName, i)
+ if err != nil {
+ return fmt.Errorf("error inserting into server_profile
table, %v", err)
+ }
+ }
+
+ err = tx.QueryRow("SELECT ARRAY_AGG(profile_name) FROM server_profile
WHERE server=$1", *id).Scan(pq.Array(&profileNames))
+ if err == sql.ErrNoRows {
+ return fmt.Errorf("selecting server_profile by profile_name: "
+ err.Error())
+ }
Review Comment:
To insert <var>n</var> Profiles, this function will make
<code><var>n</var>+2</code> (<code><var>n</var>+1</code> in the highlighted
block, which doesn't include the `DELETE` above) database queries. You can
instead make it exactly 2 queries by just doing a single insert, and reading
back the inserted values rather than making a separate query to retrieve them.
Arguably you don't need to read them back at all because, assuming the
insertion and deletion queries were both successful, the input array is exactly
the same as the output. But it is safer to read them back, to be sure the
output is _exactly_ what was inserted, so as long as performance isn't a big
deal it's not a big deal to do it.
The query for that would look something like
```sql
WITH inserted AS (
INSERT INTO public.server_profile (server, profile_name, priority)
SELECT $1, profile, priority
FROM UNNEST($2::text[], $3::int[]) AS tmp("profile", "priority")
RETURNING profile, priority
)
SELECT profile
FROM inserted
ORDER BY priority ASC
```
The `WITH` is necessary to ensure the returned rows are in order, otherwise
you could just also return the priority to Go and sort there - but that seems
like more work to me than just using the `WITH`.
This way you'd have to do a scanning loop instead of being able to cleanly
scan into a `pq.Array`-wrapped slice, but if you wanted to make the query a bit
more complex to make the Go simpler you could scan from a `QueryRow` into a
`pq.Array` using
```sql
WITH inserted AS (
INSERT INTO public.server_profile(server, profile_name, priority)
SELECT $1, profile, priority
FROM UNNEST($2::text[], $3::int[]) AS tmp("profile", "priority")
RETURNING profile, priority
)
SELECT ARRAY_AGG(profile)
FROM (
SELECT profile
FROM inserted
ORDER BY priority ASC
) AS returned(profile)
```
Either way is fine as far as I'm concerned. But I do think reducing the
number of queries to a constant is a good improvement to make.
I have some sample programs I wrote that connect to the development
environment's TODB server and demonstrate each approach using a table they make
for themselves. They're lengthy, so I've included them in collapsible boxes
below.
<details><summary>Scanning rows sequentially in a loop</summary>
```go
package main
import (
"database/sql"
"fmt"
"os"
"strings"
"github.com/lib/pq"
)
const setupquery = `
CREATE TABLE IF NOT EXISTS testquest(
server bigint,
profile text,
priority int,
CONSTRAINT uniq_server_prof UNIQUE(server, profile),
CONSTRAINT uniq_server_prio UNIQUE(server, priority)
)`
const query = `
WITH inserted AS (
INSERT INTO testquest
SELECT $1, "profile", "priority"
FROM UNNEST($2::text[], $3::int[]) AS tmp("profile", "priority")
RETURNING profile, priority
)
SELECT profile
FROM inserted
`
const teardownquery = `
DROP TABLE IF EXISTS testquest
`
func main() {
db, err := sql.Open("postgres",
"postgresql://postgres:twelve12@localhost:5432/traffic_ops_development?sslmode=disable")
if err != nil {
fmt.Fprintf(os.Stderr, "failed to connect to database: %v\n",
err)
os.Exit(1)
}
_, err = db.Exec(setupquery)
if err != nil {
fmt.Fprintf(os.Stderr, "failed to setup schema: %v\n", err)
os.Exit(1)
}
exitcode := 0
defer func() {
if _, err := db.Exec(teardownquery); err != nil {
fmt.Fprintf(os.Stderr, "failed to tear down schema:
%v\n", err)
exitcode++
}
os.Exit(exitcode)
}()
data := []string{"a", "bunch", "of", "Profile", "names"}
priorities := []int{0, 1, 2, 3, 4}
rows, err := db.Query(query, 1, pq.Array(data), pq.Array(priorities))
if err != nil {
fmt.Fprintf(os.Stderr, "Failed to run insert query: %v\n", err)
exitcode++
return
}
if err = rows.Err(); err != nil {
fmt.Fprintf(os.Stderr, "Rows have error: %v\n", err)
exitcode++
return
}
defer func() {
if err := rows.Close(); err != nil {
fmt.Fprintf(os.Stderr, "Failed to close rows: %v\n",
err)
exitcode++
}
}()
profiles := make([]string, 0, len(data))
for rows.Next() {
var profile string
if err = rows.Scan(&profile); err != nil {
fmt.Fprintf(os.Stderr, "failed to scan row: %v\n", err)
}
profiles = append(profiles, profile)
}
fmt.Println(strings.Join(profiles, ", "))
}
```
</details>
<details><summary>Scanning a single returned row into a
<code>pq.Array</code></summary>
```go
package main
import (
"database/sql"
"fmt"
"os"
"strings"
"github.com/lib/pq"
)
const setupquery = `
CREATE TABLE IF NOT EXISTS testquest(
server bigint,
profile text,
priority int,
CONSTRAINT uniq_server_prof UNIQUE(server, profile),
CONSTRAINT uniq_server_prio UNIQUE(server, priority)
)`
const query = `
WITH inserted AS (
INSERT INTO testquest
SELECT $1, "profile", "priority"
FROM UNNEST($2::text[], $3::int[]) AS tmp("profile", "priority")
RETURNING profile, priority
)
SELECT ARRAY_AGG(profile)
FROM (
SELECT profile
FROM inserted
ORDER BY priority ASC
) AS returned(profile)
`
const teardownquery = `
DROP TABLE IF EXISTS testquest
`
func main() {
db, err := sql.Open("postgres",
"postgresql://postgres:twelve12@localhost:5432/traffic_ops_development?sslmode=disable")
if err != nil {
fmt.Fprintf(os.Stderr, "failed to connect to database: %v\n",
err)
os.Exit(1)
}
_, err = db.Exec(setupquery)
if err != nil {
fmt.Fprintf(os.Stderr, "failed to setup schema: %v\n", err)
os.Exit(1)
}
exitcode := 0
defer func() {
if _, err := db.Exec(teardownquery); err != nil {
fmt.Fprintf(os.Stderr, "failed to tear down schema:
%v\n", err)
exitcode++
}
os.Exit(exitcode)
}()
data := []string{"a", "bunch", "of", "Profile", "names"}
priorities := []int{0, 1, 2, 3, 4}
profiles := make([]string, 0, len(data))
err = db.QueryRow(query, 1, pq.Array(data),
pq.Array(priorities)).Scan(pq.Array(&profiles))
if err != nil {
fmt.Fprintf(os.Stderr, "Failed to run insert query/scan
results: %v\n", err)
exitcode++
return
}
fmt.Println(strings.Join(profiles, ", "))
}
```
This one is actually quite a bit shorter. Maybe this really is the way to
go? Query is more complex, and the loop is more generic/familiar than
`pq.Array`, though.
</details>
Both programs print the profiles they insert on success (which is a static
array you can find in the source code, but for reference it's `["a", "bunch",
"of", "Profile", "names"]`, in that order).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
